Re: [PATCH 2/6] imap-send.c, cram: allocate enough memory for null terminated string

From: Jeff King <peff@peff.net>
Date: Mar 30, 2016, 01:07 UTC
Message-ID: <20160330010756.GA2237@sigill.intra.peff.net>
In-Reply-To: <1459298333-21899-3-git-send-email-sbeller@google.com>

On Tue, Mar 29, 2016 at 05:38:49PM -0700, Stefan Beller wrote:

Show 22 quoted lines

> `strlen` returns the length of a string without the terminating null byte.
> To make sure enough memory is allocated we need to pass `strlen(..) + 1`
> to the allocation function.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  imap-send.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/imap-send.c b/imap-send.c
> index 2c52027..f7e9909 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -872,7 +872,7 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)
>  	 * enough upper bound for challenge (decoded result).
>  	 */
>  	encoded_len = strlen(challenge_64);
> -	challenge = xmalloc(encoded_len);
> +	challenge = xmalloc(encoded_len + 1);
>  	decoded_len = EVP_DecodeBlock((unsigned char *)challenge,
>  				      (unsigned char *)challenge_64, encoded_len);
>  	if (decoded_len < 0)

Others pointed out that patches 1 and 2 here probably don't need the extra byte. But as a side note, for any cases that do, please use xmallocz() instead of manually adding 1 to the length. Even if you don't need the final byte to be NUL, it checks for integer overflow.

-Peff