Re: [PATCH] Fix imap_auth_gss() security level size check and buf_size type.

Alejandro Colomar via Mutt-dev <[email protected]> Sun, 19 Apr 2026 08:46:49 +0200
Newsgroups	gmane.mail.mutt.devel
Message-ID	<aeR6Q4uSH6YmyPOn@devuan>
On 2026-04-19T13:51:31+0800, Kevin J. McCarthy wrote:
> Make sure send_token.length is 4 bytes before reading the data.
> 
> Fix the buf_size type to be uint32_t instead of long.  ntohl()
> operates on, and returns, a 32 bit unsigned integer.  Most
> architectures now use a 64-bit long.
> 
> I believe this only worked because in Little-Endian, the
> least-significant bits come first, so even though we were using 8
> bytes of send_token.value (4 of which were out of bounds) for the cast
> to long, only the first 4 bytes were used to truncate to the uint32_t
> that ntohl() used.  Likewise when we converted htonl() further down.
> 
> Additionally, the comments indicate that mutt wasn't using buf_size in
> any case, so perhaps that also explains the lack of bug reports.
> 
> Thanks to [email protected] for the security report.

Reviewed-by: Alejandro Colomar <[email protected]>

> ---
> This is 6 in the list evilrabbit sent.
> 
>  imap/auth_gss.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/imap/auth_gss.c b/imap/auth_gss.c
> index 4fb6d7e1..fb9e50d8 100644
> --- a/imap/auth_gss.c
> +++ b/imap/auth_gss.c
> @@ -108,7 +108,7 @@ imap_auth_res_t imap_auth_gss (IMAP_DATA* idata, const char* method)
>    int cflags;
>    OM_uint32 maj_stat, min_stat;
>    BUFFER *buf1 = NULL, *buf2 = NULL;
> -  unsigned long buf_size;
> +  uint32_t buf_size;
>    int rc, retval = IMAP_AUTH_FAILURE;
>  
>    if (!mutt_bit_isset (idata->capabilities, AGSSAPI))
> @@ -259,6 +259,14 @@ imap_auth_res_t imap_auth_gss (IMAP_DATA* idata, const char* method)
>    }
>    dprint (2, (debugfile, "Credential exchange complete\n"));
>  
> +  if (send_token.length < 4)
> +  {
> +    /* TODO: convert to muttdbg() in master branch merge */
> +    dprint(2, (debugfile, "Truncated security level data\n"));
> +    gss_release_buffer(&min_stat, &send_token);
> +    goto err_abort_cmd;
> +  }
> +
>    /* first octet is security levels supported. We want NONE */
>  #ifdef DEBUG
>    server_conf_flags = ((char*) send_token.value)[0];
> @@ -272,7 +280,7 @@ imap_auth_res_t imap_auth_gss (IMAP_DATA* idata, const char* method)
>  
>    /* we don't care about buffer size if we don't wrap content. But here it is */
>    ((char*) send_token.value)[0] = 0;
> -  buf_size = ntohl (*((long *) send_token.value));
> +  buf_size = ntohl(*((uint32_t *) send_token.value));
>    gss_release_buffer (&min_stat, &send_token);
>    dprint (2, (debugfile, "Unwrapped security level flags: %c%c%c\n",
>                server_conf_flags & GSS_AUTH_P_NONE      ? 'N' : '-',
> -- 
> 2.53.0
> 

-- 
<https://www.alejandro-colomar.es>
signature.asc (application/pgp-signature, 833 B)