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/558a65e48e6819140b409cf58a0340aa...

Author: Carsten Bock carsten@ng-voice.com Committer: Carsten Bock carsten@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@traviscross.com

---

Modified: modules/utils/functions.c

---

Diff: https://github.com/kamailio/kamailio/commit/558a65e48e6819140b409cf58a0340aa... Patch: https://github.com/kamailio/kamailio/commit/558a65e48e6819140b409cf58a0340aa...

---

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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev