Re: [PATCH] Fix IMAP auth_crm MD5 digest of secret to use memcpy().

On Sun, Apr 19, 2026 at 07:46:02PM +0200, Alejandro Colomar via Mutt-dev wrote:
> Hi Rene,
> 
> On 2026-04-19T17:08:29+0200, Rene Kita wrote:
> > On Sun, Apr 19, 2026 at 01:50:51PM +0800, Kevin J. McCarthy wrote:
> > > For a secret longer that MD5_BLOCK_LEN, an MD5 digest is used
> > 
> > s/that/than
> > 
> > > instead.  However, mutt was incorrectly using strfcpy() on the raw
> > > binary value returned by md5_buffer, instead of memcpy().  This could
> > > result in authentication failing.
> > > 
> > > This likely hasn't been a big issue because:
> > > 1. CRAM-MD5 is not used much anymore
> > > 2. Most people likely don't have a password length greater than 64
> > >    bytes.
> > > 3. It relies on the case of an exactly aligned 0x00 byte in the digest
> > >    result, which is likely also infrequent.
> > > 
> > > Thanks to [email protected] for the security report.
> > > ---
> > > This is 5 in the list evilrabbit sent.
> > > 
> > >  imap/auth_cram.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/imap/auth_cram.c b/imap/auth_cram.c
> > > index 6080ea47..6a265de6 100644
> > > --- a/imap/auth_cram.c
> > > +++ b/imap/auth_cram.c
> > > @@ -149,7 +149,7 @@ static void hmac_md5 (const char* password, char* challenge,
> > >    if (secret_len > MD5_BLOCK_LEN)
> > >    {
> > >      md5_buffer (password, secret_len, hash_passwd);
> > > -    strfcpy ((char*) secret, (char*) hash_passwd, MD5_DIGEST_LEN);
> > > +    memcpy(secret, hash_passwd, MD5_DIGEST_LEN);
> > >      secret_len = MD5_DIGEST_LEN;
> > >    }
> > >    else
> > 
> > The part after the else above is:
> >   strfcpy((char *) secret, password, sizeof(secret));
> > 
> > Are we dealing here with strings or with buffers of bytes/chars?
> 
> Buffers of bytes.
> 
> > If we
> > need strings, that is nul-terminated char buffers, the memcpy above will
> > not ensure it, right? If we are not dealing with strings we shouldn't
> > use str-functions here to avoid confusing people like me. ;-)
> 
> Agree.  We should at least do it in the master branch.

JFTR, I was not suggesting to do a mass fix, but to do a 'while there'
fix. :-)

> > From my little understanding of the code I would suggest to use memcpy
> > but also replace the strfcpy in the else clause.
> 
> Agree.
> 
> 
> Have a lovely day!
> Alex
> 
> -- 
> <https://www.alejandro-colomar.es>