From d8b4adb59031c72386a05767f72dc30949dfddfc Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Tue, 10 Sep 2013 17:56:56 +0000 Subject: [PATCH] Fix incorrect usages of ast_realloc(). There are several locations in the code base where this is done: buf = ast_realloc(buf, new_size); This is going to leak the original buf contents if the realloc fails. Review: https://reviewboard.asterisk.org/r/2832/ ........ Merged revisions 398757 from http://svn.asterisk.org/svn/asterisk/branches/1.8 git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/11@398758 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- funcs/func_dialgroup.c | 8 ++++- main/asterisk.c | 68 ++++++++++++++++++++++++++++-------------- main/cli.c | 33 ++++++++++++++++---- main/event.c | 6 +++- main/heap.c | 13 ++++---- main/indications.c | 7 +++-- main/xmldoc.c | 13 ++++---- res/res_musiconhold.c | 16 ++++++---- 8 files changed, 115 insertions(+), 49 deletions(-) diff --git a/funcs/func_dialgroup.c b/funcs/func_dialgroup.c index 0e078cd80d..37aba6afbe 100644 --- a/funcs/func_dialgroup.c +++ b/funcs/func_dialgroup.c @@ -174,11 +174,17 @@ static int dialgroup_refreshdb(struct ast_channel *chan, const char *cdialgroup) { int len = 500, res = 0; char *buf = NULL; + char *new_buf; char *dialgroup = ast_strdupa(cdialgroup); do { len *= 2; - buf = ast_realloc(buf, len); + new_buf = ast_realloc(buf, len); + if (!new_buf) { + ast_free(buf); + return -1; + } + buf = new_buf; if ((res = dialgroup_read(chan, "", dialgroup, buf, len)) < 0) { ast_free(buf); diff --git a/main/asterisk.c b/main/asterisk.c index 0121e05759..83e0a67762 100644 --- a/main/asterisk.c +++ b/main/asterisk.c @@ -2640,45 +2640,62 @@ static char *cli_prompt(EditLine *editline) return ast_str_buffer(prompt); } +static void destroy_match_list(char **match_list, int matches) +{ + if (match_list) { + int idx; + + for (idx = 0; idx < matches; ++idx) { + ast_free(match_list[idx]); + } + ast_free(match_list); + } +} + static char **ast_el_strtoarr(char *buf) { - char **match_list = NULL, **match_list_tmp, *retstr; - size_t match_list_len; + char *retstr; + char **match_list = NULL; + char **new_list; + size_t match_list_len = 1; int matches = 0; - match_list_len = 1; - while ( (retstr = strsep(&buf, " ")) != NULL) { - - if (!strcmp(retstr, AST_CLI_COMPLETE_EOF)) + while ((retstr = strsep(&buf, " "))) { + if (!strcmp(retstr, AST_CLI_COMPLETE_EOF)) { break; + } if (matches + 1 >= match_list_len) { match_list_len <<= 1; - if ((match_list_tmp = ast_realloc(match_list, match_list_len * sizeof(char *)))) { - match_list = match_list_tmp; - } else { - if (match_list) - ast_free(match_list); - return (char **) NULL; + new_list = ast_realloc(match_list, match_list_len * sizeof(char *)); + if (!new_list) { + destroy_match_list(match_list, matches); + return NULL; } + match_list = new_list; } - match_list[matches++] = ast_strdup(retstr); + retstr = ast_strdup(retstr); + if (!retstr) { + destroy_match_list(match_list, matches); + return NULL; + } + match_list[matches++] = retstr; } - if (!match_list) - return (char **) NULL; + if (!match_list) { + return NULL; + } if (matches >= match_list_len) { - if ((match_list_tmp = ast_realloc(match_list, (match_list_len + 1) * sizeof(char *)))) { - match_list = match_list_tmp; - } else { - if (match_list) - ast_free(match_list); - return (char **) NULL; + new_list = ast_realloc(match_list, (match_list_len + 1) * sizeof(char *)); + if (!new_list) { + destroy_match_list(match_list, matches); + return NULL; } + match_list = new_list; } - match_list[matches] = (char *) NULL; + match_list[matches] = NULL; return match_list; } @@ -2779,7 +2796,9 @@ static char *cli_complete(EditLine *editline, int ch) if (nummatches > 0) { char *mbuf; + char *new_mbuf; int mlen = 0, maxmbuf = 2048; + /* Start with a 2048 byte buffer */ if (!(mbuf = ast_malloc(maxmbuf))) { *((char *) lf->cursor) = savechr; @@ -2793,10 +2812,13 @@ static char *cli_complete(EditLine *editline, int ch) if (mlen + 1024 > maxmbuf) { /* Every step increment buffer 1024 bytes */ maxmbuf += 1024; - if (!(mbuf = ast_realloc(mbuf, maxmbuf))) { + new_mbuf = ast_realloc(mbuf, maxmbuf); + if (!new_mbuf) { + ast_free(mbuf); *((char *) lf->cursor) = savechr; return (char *)(CC_ERROR); } + mbuf = new_mbuf; } /* Only read 1024 bytes at a time */ res = read(ast_consock, mbuf + mlen, 1024); diff --git a/main/cli.c b/main/cli.c index 5e6263d386..aeec29998c 100644 --- a/main/cli.c +++ b/main/cli.c @@ -2370,9 +2370,22 @@ int ast_cli_generatornummatches(const char *text, const char *word) return matches; } +static void destroy_match_list(char **match_list, int matches) +{ + if (match_list) { + int idx; + + for (idx = 1; idx < matches; ++idx) { + ast_free(match_list[idx]); + } + ast_free(match_list); + } +} + char **ast_cli_completion_matches(const char *text, const char *word) { char **match_list = NULL, *retstr, *prevstr; + char **new_list; size_t match_list_len, max_equal, which, i; int matches = 0; @@ -2381,14 +2394,19 @@ char **ast_cli_completion_matches(const char *text, const char *word) while ((retstr = ast_cli_generator(text, word, matches)) != NULL) { if (matches + 1 >= match_list_len) { match_list_len <<= 1; - if (!(match_list = ast_realloc(match_list, match_list_len * sizeof(*match_list)))) + new_list = ast_realloc(match_list, match_list_len * sizeof(*match_list)); + if (!new_list) { + destroy_match_list(match_list, matches); return NULL; + } + match_list = new_list; } match_list[++matches] = retstr; } - if (!match_list) + if (!match_list) { return match_list; /* NULL */ + } /* Find the longest substring that is common to all results * (it is a candidate for completion), and store a copy in entry 0. @@ -2401,20 +2419,23 @@ char **ast_cli_completion_matches(const char *text, const char *word) max_equal = i; } - if (!(retstr = ast_malloc(max_equal + 1))) { - ast_free(match_list); + retstr = ast_malloc(max_equal + 1); + if (!retstr) { + destroy_match_list(match_list, matches); return NULL; } - ast_copy_string(retstr, match_list[1], max_equal + 1); match_list[0] = retstr; /* ensure that the array is NULL terminated */ if (matches + 1 >= match_list_len) { - if (!(match_list = ast_realloc(match_list, (match_list_len + 1) * sizeof(*match_list)))) { + new_list = ast_realloc(match_list, (match_list_len + 1) * sizeof(*match_list)); + if (!new_list) { ast_free(retstr); + destroy_match_list(match_list, matches); return NULL; } + match_list = new_list; } match_list[matches + 1] = NULL; diff --git a/main/event.c b/main/event.c index 79f894db5b..0904f8b615 100644 --- a/main/event.c +++ b/main/event.c @@ -1197,13 +1197,17 @@ int ast_event_append_ie_raw(struct ast_event **event, enum ast_event_ie_type ie_ const void *data, size_t data_len) { struct ast_event_ie *ie; + struct ast_event *old_event; unsigned int extra_len; uint16_t event_len; event_len = ntohs((*event)->event_len); extra_len = sizeof(*ie) + data_len; - if (!(*event = ast_realloc(*event, event_len + extra_len))) { + old_event = *event; + *event = ast_realloc(*event, event_len + extra_len); + if (!*event) { + ast_free(old_event); return -1; } diff --git a/main/heap.c b/main/heap.c index b2c0d38359..c04f7a0103 100644 --- a/main/heap.c +++ b/main/heap.c @@ -181,18 +181,19 @@ static int grow_heap(struct ast_heap *h #endif ) { - h->avail_len = h->avail_len * 2 + 1; + void **new_heap; + size_t new_len = h->avail_len * 2 + 1; - if (!(h->heap = #ifdef MALLOC_DEBUG - __ast_realloc(h->heap, h->avail_len * sizeof(void *), file, lineno, func) + new_heap = __ast_realloc(h->heap, new_len * sizeof(void *), file, lineno, func); #else - ast_realloc(h->heap, h->avail_len * sizeof(void *)) + new_heap = ast_realloc(h->heap, new_len * sizeof(void *)); #endif - )) { - h->cur_len = h->avail_len = 0; + if (!new_heap) { return -1; } + h->heap = new_heap; + h->avail_len = new_len; return 0; } diff --git a/main/indications.c b/main/indications.c index bd1df8f8c8..750198dc7b 100644 --- a/main/indications.c +++ b/main/indications.c @@ -341,12 +341,12 @@ int ast_playtones_start(struct ast_channel *chan, int vol, const char *playlst, } while ((s = strsep(&stringp, separator)) && !ast_strlen_zero(s)) { + struct playtones_item *new_items; struct ast_tone_zone_part tone_data = { .time = 0, }; s = ast_strip(s); - if (s[0]=='!') { s++; } else if (d.reppos == -1) { @@ -374,9 +374,12 @@ int ast_playtones_start(struct ast_channel *chan, int vol, const char *playlst, } } - if (!(d.items = ast_realloc(d.items, (d.nitems + 1) * sizeof(*d.items)))) { + new_items = ast_realloc(d.items, (d.nitems + 1) * sizeof(*d.items)); + if (!new_items) { + ast_free(d.items); return -1; } + d.items = new_items; d.items[d.nitems].fac1 = 2.0 * cos(2.0 * M_PI * (tone_data.freq1 / sample_rate)) * max_sample_val; d.items[d.nitems].init_v2_1 = sin(-4.0 * M_PI * (tone_data.freq1 / sample_rate)) * d.vol; diff --git a/main/xmldoc.c b/main/xmldoc.c index 18afbb6368..a0a417c66c 100644 --- a/main/xmldoc.c +++ b/main/xmldoc.c @@ -578,8 +578,11 @@ static struct ast_xml_node *xmldoc_get_node(const char *type, const char *name, */ static void __attribute__((format(printf, 4, 5))) xmldoc_reverse_helper(int reverse, int *len, char **syntax, const char *fmt, ...) { - int totlen, tmpfmtlen; - char *tmpfmt, tmp; + int totlen; + int tmpfmtlen; + char *tmpfmt; + char *new_syntax; + char tmp; va_list ap; va_start(ap, fmt); @@ -592,12 +595,12 @@ static void __attribute__((format(printf, 4, 5))) xmldoc_reverse_helper(int reve tmpfmtlen = strlen(tmpfmt); totlen = *len + tmpfmtlen + 1; - *syntax = ast_realloc(*syntax, totlen); - - if (!*syntax) { + new_syntax = ast_realloc(*syntax, totlen); + if (!new_syntax) { ast_free(tmpfmt); return; } + *syntax = new_syntax; if (reverse) { memmove(*syntax + tmpfmtlen, *syntax, *len); diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c index 774bd5c040..63e074bb54 100644 --- a/res/res_musiconhold.c +++ b/res/res_musiconhold.c @@ -1047,20 +1047,26 @@ static struct ast_generator mohgen = { static int moh_add_file(struct mohclass *class, const char *filepath) { if (!class->allowed_files) { - if (!(class->filearray = ast_calloc(1, INITIAL_NUM_FILES * sizeof(*class->filearray)))) + class->filearray = ast_calloc(1, INITIAL_NUM_FILES * sizeof(*class->filearray)); + if (!class->filearray) { return -1; + } class->allowed_files = INITIAL_NUM_FILES; } else if (class->total_files == class->allowed_files) { - if (!(class->filearray = ast_realloc(class->filearray, class->allowed_files * sizeof(*class->filearray) * 2))) { - class->allowed_files = 0; - class->total_files = 0; + char **new_array; + + new_array = ast_realloc(class->filearray, class->allowed_files * sizeof(*class->filearray) * 2); + if (!new_array) { return -1; } + class->filearray = new_array; class->allowed_files *= 2; } - if (!(class->filearray[class->total_files] = ast_strdup(filepath))) + class->filearray[class->total_files] = ast_strdup(filepath); + if (!class->filearray[class->total_files]) { return -1; + } class->total_files++; -- 2.47.2