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? 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. ;-)

>From my little understanding of the code I would suggest to use memcpy
but also replace the strfcpy in the else clause.

Reply via email to