Re: Fwd: report 3/3: latent unterminated buffer in imap_wordcasecmp()

"Kevin J. McCarthy" <[email protected]> Fri, 19 Jun 2026 13:02:17 +0800

Newsgroups	gmane.mail.mutt.devel
Message-ID	<[email protected]>

On Thu, Jun 18, 2026 at 02:46:50PM +0200, Alejandro Colomar via Mutt-dev wrote:
>On 2026-06-18T14:43:52+0200, Alejandro Colomar wrote:
>> TBH, it still reads a bit too complex to me.  I struggle to fully
>> understand it.
>>
>> I've tried rewriting the function from scratch, to see if I understand
>> it correctly.  Is this rewrite correct?
>>
>> 	@@ -884,20 +884,10 @@ void imap_unmunge_mbox_name(IMAP_DATA *idata, char *s)
>> 	 int imap_wordcasecmp(const char *a, const char *b)
>> 	 {
>> 	   char tmp[SHORT_STRING];
>> 	-  const char *s = b;
>> 	-  int i;
>> 	
>> 	-  tmp[SHORT_STRING-1] = 0;
>> 	-  for (i=0;i < SHORT_STRING-2;i++,s++)
>> 	-  {
>> 	-    if (!*s || IS_ASCII_WS(*s))
>> 	-    {
>> 	-      tmp[i] = 0;
>> 	-      break;
>> 	-    }
>> 	-    tmp[i] = *s;
>> 	-  }
>> 	-  tmp[i+1] = 0;
>> 	+  len = strspn(b, " \t\n\v\f\r");  // We could #define ASCII_WS " \t\n\v\f\r"
>
>Actually, I meant strcspn().
>
>And I forgot to declare len as size_t.
>
>> 	+  len = MIN(len, sizeof(tmp)-1);  // Is truncation a problem?!
>> 	+  strcpy(mempcpy(tmp, b, len), "");

Seems okay for (fixing and ) duplicating the functionality.

Currently, truncation of b isn't a problem because all of the "a" 
parameters are in the Capabilities array, and aren't even close to the 
size SHORT_STRING, so any "b" value of size 127 is not going to match an 
"a" parameter.

However, this "utility" function is pretty lousy, and as Ian mentions I 
can't think of any reason to copy b over to a tmp[] array.

If we're going to rewrite it, why don't we just compute length of a, 
grab the strcpn() retval as length of b, and if they match then just use 
ascii_strncmp()?

-- 
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA

signature.asc (application/pgp-signature, 833 B)