From: Matthew Jordan Date: Wed, 2 Jan 2013 16:00:06 +0000 (+0000) Subject: Resolve crashes due to large stack allocations when using TCP X-Git-Tag: certified/1.8.15-cert2~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=21288e6e248f122f49b56a389caf201b2c499ca3;p=thirdparty%2Fasterisk.git Resolve crashes due to large stack allocations when using TCP Asterisk had several places where messages received over various network transports may be copied in a single stack allocation. In the case of TCP, since multiple packets in a stream may be concatenated together, this can lead to large allocations that overflow the stack. This patch modifies those portions of Asterisk using TCP to either favor heap allocations or use an upper bound to ensure that the stack will not overflow: * For HTTP, the allocation is now a heap allocation instead of a stack allocation * For XMPP (in res_jabber), the allocation has been eliminated since it was unnecesary. Note that the HTTP portion of this issue was independently found by Brandon Edwards of Exodus Intelligence. (issue ASTERISK-20658) Reported by: wdoekes, Brandon Edwards Tested by: mmichelson, wdoekes patches: ASTERISK-20658_res_jabber.c.patch uploaded by mmichelson (license 5049) issueA20658_http_postvars_use_malloc2.patch uploaded by wdoekes (license 5674) ........ Merged revisions 378269 from http://svn.asterisk.org/svn/asterisk/branches/1.8 git-svn-id: https://origsvn.digium.com/svn/asterisk/certified/branches/1.8.15@378290 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/main/http.c b/main/http.c index 8fc5e320b6..2ed8050c19 100644 --- a/main/http.c +++ b/main/http.c @@ -622,6 +622,7 @@ struct ast_variable *ast_http_get_post_vars( int content_length = 0; struct ast_variable *v, *post_vars=NULL, *prev = NULL; char *buf, *var, *val; + int res; for (v = headers; v; v = v->next) { if (!strcasecmp(v->name, "Content-Type")) { @@ -634,21 +635,27 @@ struct ast_variable *ast_http_get_post_vars( for (v = headers; v; v = v->next) { if (!strcasecmp(v->name, "Content-Length")) { - content_length = atoi(v->value) + 1; + content_length = atoi(v->value); break; } } - if (!content_length) { + if (content_length <= 0) { return NULL; } - if (!(buf = alloca(content_length))) { + buf = ast_malloc(content_length + 1); + if (!buf) { return NULL; } - if (!fgets(buf, content_length, ser->f)) { - return NULL; + + res = fread(buf, 1, content_length, ser->f); + if (res < content_length) { + /* Error, distinguishable by ferror() or feof(), but neither + * is good. */ + goto done; } + buf[content_length] = '\0'; while ((val = strsep(&buf, "&"))) { var = strsep(&val, "="); @@ -667,6 +674,9 @@ struct ast_variable *ast_http_get_post_vars( prev = v; } } + +done: + ast_free(buf); return post_vars; } diff --git a/res/res_jabber.c b/res/res_jabber.c index 3ef99815a3..502083fb6f 100644 --- a/res/res_jabber.c +++ b/res/res_jabber.c @@ -777,7 +777,7 @@ static struct ast_custom_function jabberstatus_function = { */ static int acf_jabberreceive_read(struct ast_channel *chan, const char *name, char *data, char *buf, size_t buflen) { - char *aux = NULL, *parse = NULL; + char *parse = NULL; int timeout; int jidlen, resourcelen; struct timeval start; @@ -894,7 +894,7 @@ static int acf_jabberreceive_read(struct ast_channel *chan, const char *name, ch continue; } found = 1; - aux = ast_strdupa(tmp->message); + ast_copy_string(buf, tmp->message, buflen); AST_LIST_REMOVE_CURRENT(list); aji_message_destroy(tmp); break; @@ -919,7 +919,6 @@ static int acf_jabberreceive_read(struct ast_channel *chan, const char *name, ch ast_log(LOG_NOTICE, "Timed out : no message received from %s\n", args.jid); return -1; } - ast_copy_string(buf, aux, buflen); return 0; }