[sr-dev] git:4.3:558a65e4: utils: Fix buffer overflow; do not NULL-terminate HTTP result

[Daniel-Constantin Mierla]

Somehow not clear for me at first sight -- the allocation adds +1 to
keep the terminating NULL char, so when size*nmemb == 1, then the
allocated size should be 2.

A while ago, while looking at the code to investigate an issue, I wrote
to sr-dev that this allocation keeps adding 1 extra char for each
write_function callback, while only 1 will be needed to store the last NULL.

The main concern with this change is if there is any side effect by
removing the ending NULL.

Cheers,
Daniel

On 28/08/15 12:12, Carsten Bock wrote:
> Module: kamailio
> Branch: 4.3
> Commit: 558a65e48e6819140b409cf58a0340aa78b8c2cf
> URL: https://github.com/kamailio/kamailio/commit/558a65e48e6819140b409cf58a0340aa78b8c2cf
>
> Author: Carsten Bock <carsten at ng-voice.com>
> Committer: Carsten Bock <carsten at ng-voice.com>
> Date: 2015-08-28T12:12:34+02:00
>
> utils: Fix buffer overflow; do not NULL-terminate HTTP result
>
> Fix buffer overflow in the `write_function` that takes the resulting
> data from libcurl. The function was trying to NULL terminate the
> string, but this could result in overwriting the buffer by one byte
> when size*nmemb == 1.
> This also caused some memory corruptions, reported on sr-dev.
>
> Reported by: Travis Cross <tc at traviscross.com>
>
> ---
>
> Modified: modules/utils/functions.c
>
> ---
>
> Diff:  https://github.com/kamailio/kamailio/commit/558a65e48e6819140b409cf58a0340aa78b8c2cf.diff
> Patch: https://github.com/kamailio/kamailio/commit/558a65e48e6819140b409cf58a0340aa78b8c2cf.patch
>
> ---
>
> diff --git a/modules/utils/functions.c b/modules/utils/functions.c
> index d32d970..f261cc0 100644
> --- a/modules/utils/functions.c
> +++ b/modules/utils/functions.c
> @@ -2,7 +2,7 @@
>   * script functions of utils module
>   *
>   * Copyright (C) 2008 Juha Heinanen
> - * Copyright (C) 2013 Carsten Bock, ng-voice GmbH
> + * Copyright (C) 2013-2015 Carsten Bock, ng-voice GmbH
>   *
>   * This file is part of Kamailio, a free SIP server.
>   *
> @@ -55,7 +55,7 @@ size_t write_function( void *ptr, size_t size, size_t nmemb, void *stream_ptr)
>  	http_res_stream_t *stream = (http_res_stream_t *) stream_ptr;
>  
>  	stream->buf = (char *) pkg_realloc(stream->buf, stream->curr_size + 
> -			(size * nmemb) + 1);
> +			(size * nmemb));
>  
>  	if (stream->buf == NULL) {
>  		LM_ERR("cannot allocate memory for stream\n");
> @@ -64,15 +64,12 @@ size_t write_function( void *ptr, size_t size, size_t nmemb, void *stream_ptr)
>  
>  	memcpy(&stream->buf[stream->pos], (char *) ptr, (size * nmemb));
>  
> -	stream->curr_size += ((size * nmemb) + 1);
> +	stream->curr_size += (size * nmemb);
>  	stream->pos += (size * nmemb);
>  
> -	stream->buf[stream->pos + 1] = '\0';
> -
>  	return size * nmemb;
>  }
>  
> -
>  /* 
>   * Performs http_query and saves possible result (first body line of reply)
>   * to pvar.
>
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Book: SIP Routing With Kamailio - http://www.asipto.com