The copy_request function did not take into account the necessary null terminator

for the string to be copied into. This resulted in parse_request reading invalid
memory beyond the end of the string, and in some cases led to crashes. Thanks
to falves11 for providing the valgrind output which led to the closure of this issue.

(closes issue #12284)
Reported by: falves11

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@111662 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index fc176db..6c7478c 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -8308,15 +8308,15 @@ static void copy_request(struct sip_request *dst, const struct sip_request *src)
        if (!dst->data && !(dst->data = ast_str_create(src->data->used)))
                return;
        else if (dst->data->len < src->data->used)
-               ast_str_make_space(&dst->data, src->data->used);
+               ast_str_make_space(&dst->data, src->data->used + 1); /* Account for null terminator needed */
                
-       memcpy(dst->data->str, src->data->str, src->data->used);
+       ast_copy_string(dst->data->str, src->data->str, dst->data->len);
        dst->data->used = src->data->used;
        offset = ((void *)dst->data->str) - ((void *)src->data->str);
        /* Now fix pointer arithmetic */
-       for (x=0; x < src->headers; x++)
+       for (x = 0; x < src->headers; x++)
                dst->header[x] += offset;
-       for (x=0; x < src->lines; x++)
+       for (x = 0; x < src->lines; x++)
                dst->line[x] += offset;
        /* On some occasions this function is called without parse_request being called first so lets not create an invalid pointer */
        if (src->rlPart1)

diff --git a/include/asterisk/strings.h b/include/asterisk/strings.h
index 226c7bb..fbff083 100644 (file)
--- a/include/asterisk/strings.h
+++ b/include/asterisk/strings.h
@@ -326,7 +326,7 @@ int ast_get_timeval(const char *src, struct timeval *tv, struct timeval _default
  */
 struct ast_str {
        size_t len;     /*!< The current maximum length of the string */
-       size_t used;    /*!< Amount of space used */
+       size_t used;    /*!< Amount of space used. Does not include string's null terminator */
        struct ast_threadstorage *ts;   /*!< What kind of storage is this ? */
 #define DS_MALLOC      ((struct ast_threadstorage *)1)
 #define DS_ALLOCA      ((struct ast_threadstorage *)2)