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 SIP, the allocation now has an upper limit
 * 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)
  issueA20658_limit_sip_packet_size3.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/branches/10@378286 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index b7ca83b0d68b9aa4d3e0ecbaa53bc2520f3efcf1..0a5c6ba7dc101f18359e786075316f01ee30d3ff 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -2541,19 +2541,20 @@ static int sip_tls_read(struct sip_request *req, struct sip_request *reqcpy, str
                        int authenticated, time_t start, struct sip_threadinfo *me)
 {
        int res, content_length, after_poll = 1, need_poll = 1;
+       size_t datalen = ast_str_strlen(req->data);
        char buf[1024] = "";
        int timeout = -1;
-
-       /* Read in headers one line at a time */
-       while (ast_str_strlen(req->data) < 4 || strncmp(REQ_OFFSET_TO_STR(req, data->used - 4), "\r\n\r\n", 4)) {
-               if (!tcptls_session->client && !authenticated) {
-                       if ((timeout = sip_check_authtimeout(start)) < 0) {
-                               ast_debug(2, "SIP SSL server failed to determine authentication timeout\n");
+ 
+       /* Read in headers one line at a time */
+       while (datalen < 4 || strncmp(REQ_OFFSET_TO_STR(req, data->used - 4), "\r\n\r\n", 4)) {
+               if (!tcptls_session->client && !authenticated) {
+                       if ((timeout = sip_check_authtimeout(start)) < 0) {
+                               ast_debug(2, "SIP TLS server failed to determine authentication timeout\n");
                                return -1;
                        }
 
                        if (timeout == 0) {
-                               ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "SSL": "TCP");
+                               ast_debug(2, "SIP TLS server timed out\n");
                                return -1;
                        }
                } else {
@@ -2568,11 +2569,11 @@ static int sip_tls_read(struct sip_request *req, struct sip_request *reqcpy, str
                        after_poll = 1;
                        res = ast_wait_for_input(tcptls_session->fd, timeout);
                        if (res < 0) {
-                               ast_debug(2, "SIP TCP server :: ast_wait_for_input returned %d\n", res);
+                               ast_debug(2, "SIP TLS server :: ast_wait_for_input returned %d\n", res);
                                return -1;
                        } else if (res == 0) {
                                /* timeout */
-                               ast_debug(2, "SIP TCP server timed out\n");
+                               ast_debug(2, "SIP TLS server timed out\n");
                                return -1;
                        }
                }
@@ -2593,6 +2594,13 @@ static int sip_tls_read(struct sip_request *req, struct sip_request *reqcpy, str
                        return -1;
                }
                ast_str_append(&req->data, 0, "%s", buf);
+
+               datalen = ast_str_strlen(req->data);
+               if (datalen > SIP_MAX_PACKET_SIZE) {
+                       ast_log(LOG_WARNING, "Rejecting TLS packet from '%s' because way too large: %zu\n",
+                               ast_sockaddr_stringify(&tcptls_session->remote_address), datalen);
+                       return -1;
+               }
        }
        copy_request(reqcpy, req);
        parse_request(reqcpy);
@@ -2606,7 +2614,7 @@ static int sip_tls_read(struct sip_request *req, struct sip_request *reqcpy, str
                                }
 
                                if (timeout == 0) {
-                                       ast_debug(2, "SIP SSL server timed out\n");
+                                       ast_debug(2, "SIP TLS server timed out\n");
                                        return -1;
                                }
                        } else {
@@ -2618,11 +2626,11 @@ static int sip_tls_read(struct sip_request *req, struct sip_request *reqcpy, str
                                after_poll = 1;
                                res = ast_wait_for_input(tcptls_session->fd, timeout);
                                if (res < 0) {
-                                       ast_debug(2, "SIP TCP server :: ast_wait_for_input returned %d\n", res);
+                                       ast_debug(2, "SIP TLS server :: ast_wait_for_input returned %d\n", res);
                                        return -1;
                                } else if (res == 0) {
                                        /* timeout */
-                                       ast_debug(2, "SIP TCP server timed out\n");
+                                       ast_debug(2, "SIP TLS server timed out\n");
                                        return -1;
                                }
                        }
@@ -2645,6 +2653,13 @@ static int sip_tls_read(struct sip_request *req, struct sip_request *reqcpy, str
                        }
                        content_length -= strlen(buf);
                        ast_str_append(&req->data, 0, "%s", buf);
+               
+                       datalen = ast_str_strlen(req->data);
+                       if (datalen > SIP_MAX_PACKET_SIZE) {
+                               ast_log(LOG_WARNING, "Rejecting TLS packet from '%s' because way too large: %zu\n",
+                                       ast_sockaddr_stringify(&tcptls_session->remote_address), datalen);
+                               return -1;
+                       }
                }
        }
        /*! \todo XXX If there's no Content-Length or if the content-length and what
@@ -2818,6 +2833,8 @@ static int sip_tcp_read(struct sip_request *req, struct ast_tcptls_session_insta
        enum message_integrity message_integrity = MESSAGE_FRAGMENT;
 
        while (message_integrity == MESSAGE_FRAGMENT) {
+               size_t datalen;
+
                if (ast_str_strlen(tcptls_session->overflow_buf) == 0) {
                        char readbuf[4097];
                        int timeout;
@@ -2857,6 +2874,13 @@ static int sip_tcp_read(struct sip_request *req, struct ast_tcptls_session_insta
                        ast_str_append(&req->data, 0, "%s", ast_str_buffer(tcptls_session->overflow_buf));
                        ast_str_reset(tcptls_session->overflow_buf);
                }
+               
+               datalen = ast_str_strlen(req->data);
+               if (datalen > SIP_MAX_PACKET_SIZE) {
+                       ast_log(LOG_WARNING, "Rejecting TCP packet from '%s' because way too large: %zu\n",
+                               ast_sockaddr_stringify(&tcptls_session->remote_address), datalen);
+                       return -1;
+               }
 
                message_integrity = check_message_integrity(&req->data, &tcptls_session->overflow_buf);
        }
@@ -2928,7 +2952,7 @@ static void *_sip_tcp_helper_thread(struct ast_tcptls_session_instance *tcptls_s
        }
 
        me->threadid = pthread_self();
-       ast_debug(2, "Starting thread for %s server\n", tcptls_session->ssl ? "SSL" : "TCP");
+       ast_debug(2, "Starting thread for %s server\n", tcptls_session->ssl ? "TLS" : "TCP");
 
        /* set up pollfd to watch for reads on both the socket and the alert_pipe */
        fds[0].fd = tcptls_session->fd;
@@ -2962,7 +2986,7 @@ static void *_sip_tcp_helper_thread(struct ast_tcptls_session_instance *tcptls_s
                        }
 
                        if (timeout == 0) {
-                               ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "SSL": "TCP");
+                               ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "TLS": "TCP");
                                goto cleanup;
                        }
                } else {
@@ -2972,11 +2996,11 @@ static void *_sip_tcp_helper_thread(struct ast_tcptls_session_instance *tcptls_s
                if (ast_str_strlen(tcptls_session->overflow_buf) == 0) {
                        res = ast_poll(fds, 2, timeout); /* polls for both socket and alert_pipe */
                        if (res < 0) {
-                               ast_debug(2, "SIP %s server :: ast_wait_for_input returned %d\n", tcptls_session->ssl ? "SSL": "TCP", res);
+                               ast_debug(2, "SIP %s server :: ast_wait_for_input returned %d\n", tcptls_session->ssl ? "TLS": "TCP", res);
                                goto cleanup;
                        } else if (res == 0) {
                                /* timeout */
-                               ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "SSL": "TCP");
+                               ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "TLS": "TCP");
                                goto cleanup;
                        }
                }
@@ -3058,7 +3082,7 @@ static void *_sip_tcp_helper_thread(struct ast_tcptls_session_instance *tcptls_s
                }
        }
 
-       ast_debug(2, "Shutting down thread for %s server\n", tcptls_session->ssl ? "SSL" : "TCP");
+       ast_debug(2, "Shutting down thread for %s server\n", tcptls_session->ssl ? "TLS" : "TCP");
 
 cleanup:
        if (tcptls_session && !tcptls_session->client && !authenticated) {

diff --git a/channels/sip/include/sip.h b/channels/sip/include/sip.h
index 37554805493c517e4304ca4fd7fa49b1ee6ffddf..b5cc76d26102a11e5807fde2aaa93c1b263eb3bc 100644 (file)
--- a/channels/sip/include/sip.h
+++ b/channels/sip/include/sip.h
@@ -97,6 +97,7 @@
 
 #define SIP_MAX_HEADERS           64     /*!< Max amount of SIP headers to read */
 #define SIP_MAX_LINES             256    /*!< Max amount of lines in SIP attachment (like SDP) */
+#define SIP_MAX_PACKET_SIZE       20480  /*!< Max SIP packet size */
 #define SIP_MIN_PACKET            4096   /*!< Initialize size of memory to allocate for packets */
 #define MAX_HISTORY_ENTRIES              50     /*!< Max entires in the history list for a sip_pvt */
 

diff --git a/main/http.c b/main/http.c
index 20bc515c3a8a2eb0070c79d954a3e4e270605385..6f6bde80e4abb98620851c68513e9cf17f3a83ee 100644 (file)
--- a/main/http.c
+++ b/main/http.c
@@ -603,6 +603,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")) {
@@ -615,20 +616,28 @@ 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;
        }
 
-       buf = ast_alloca(content_length);
-       if (!fgets(buf, content_length, ser->f)) {
+       buf = ast_malloc(content_length + 1);
+       if (!buf) {
                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, "=");
                if (val) {
@@ -646,6 +655,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 92ff98cf45ab44cab57faa5a6ec99e3e681f1279..7af687d3969e100b5994c9ab73205e03a746b180 100644 (file)
--- a/res/res_jabber.c
+++ b/res/res_jabber.c
@@ -774,7 +774,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;
@@ -891,7 +891,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;
@@ -916,7 +916,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;
 }