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

Alejandro Colomar via Mutt-dev <[email protected]> Sun, 19 Apr 2026 19:46:02 +0200

Newsgroups	gmane.mail.mutt.devel
Message-ID	<aeUUCrWoPYRTRVOQ@devuan>

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.

> 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>

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