Multiple revisions 394899,410380

........
  r394899 | mjordan | 2013-07-20 22:09:16 -0500 (Sat, 20 Jul 2013) | 20 lines

  Tolerate presence of RFC2965 Cookie2 header by ignoring it

  This patch modifies parsing of cookies in Asterisk's http server by doing an
  explicit comparison of the "Cookie" header instead of looking at the first
  6 characters to determine if the header is a cookie header. This avoids
  parsing "Cookie2" headers and overwriting the previously parsed "Cookie"
  header.

  Note that we probably should be appending the cookies in each "Cookie"
  header to the parsed results; however, while clients can send multiple
  cookie headers they never really do. While this patch doesn't improve
  Asterisk's behavior in that regard, it shouldn't make it any worse either.

  Note that the solution in this patch was pointed out on the issue by the
  issue reporter, Stuart Henderson.

  (closes issue ASTERISK-21789)
  Reported by: Stuart Henderson
  Tested by: mjordan, Stuart Henderson
........
  r410380 | rmudgett | 2014-03-10 12:00:32 -0500 (Mon, 10 Mar 2014) | 11 lines

  AST-2014-001: Stack overflow in HTTP processing of Cookie headers.

  Sending a HTTP request that is handled by Asterisk with a large number of
  Cookie headers could overflow the stack.

  Another vulnerability along similar lines is any HTTP request with a
  ridiculous number of headers in the request could exhaust system memory.

  (closes issue ASTERISK-23340)
  Reported by: Lucas Molas, researcher at Programa STIC, Fundacion; and Dr. Manuel Sadosky, Buenos Aires, Argentina
........

Merged revisions 394899,410380 from http://svn.asterisk.org/svn/asterisk/branches/1.8

git-svn-id: https://origsvn.digium.com/svn/asterisk/certified/branches/1.8.15@410427 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/main/http.c b/main/http.c
index ff387f86df1b305e5aba350c829f03a98a555fbd..882f83b996afa925f982010b86d20d1b248d9313 100644 (file)
--- a/main/http.c
+++ b/main/http.c
@@ -187,9 +187,7 @@ uint32_t ast_http_manid_from_vars(struct ast_variable *headers)
                        break;
                }
        }
-       if (cookies) {
-               ast_variables_destroy(cookies);
-       }
+       ast_variables_destroy(cookies);
        return mngid;
 }
 
@@ -683,7 +681,7 @@ struct ast_variable *ast_http_get_post_vars(
                        prev = v;
                }
        }
-       
+
 done:
        ast_free(buf);
        return post_vars;
@@ -822,12 +820,13 @@ static int ssl_close(void *cookie)
 }*/
 #endif /* DO_SSL */
 
-static struct ast_variable *parse_cookies(char *cookies)
+static struct ast_variable *parse_cookies(const char *cookies)
 {
+       char *parse = ast_strdupa(cookies);
        char *cur;
        struct ast_variable *vars = NULL, *var;
 
-       while ((cur = strsep(&cookies, ";"))) {
+       while ((cur = strsep(&parse, ";"))) {
                char *name, *val;
 
                name = val = cur;
@@ -857,21 +856,19 @@ static struct ast_variable *parse_cookies(char *cookies)
 /* get cookie from Request headers */
 struct ast_variable *ast_http_get_cookies(struct ast_variable *headers)
 {
-       struct ast_variable *v, *cookies=NULL;
+       struct ast_variable *v, *cookies = NULL;
 
        for (v = headers; v; v = v->next) {
-               if (!strncasecmp(v->name, "Cookie", 6)) {
-                       char *tmp = ast_strdupa(v->value);
-                       if (cookies) {
-                               ast_variables_destroy(cookies);
-                       }
-
-                       cookies = parse_cookies(tmp);
+               if (!strcasecmp(v->name, "Cookie")) {
+                       ast_variables_destroy(cookies);
+                       cookies = parse_cookies(v->value);
                }
        }
        return cookies;
 }
 
+/*! Limit the number of request headers in case the sender is being ridiculous. */
+#define MAX_HTTP_REQUEST_HEADERS       100
 
 static void *httpd_helper_thread(void *data)
 {
@@ -882,6 +879,7 @@ static void *httpd_helper_thread(void *data)
        struct ast_variable *tail = headers;
        char *uri, *method;
        enum ast_http_method http_method = AST_HTTP_UNKNOWN;
+       int remaining_headers;
 
        if (ast_atomic_fetchadd_int(&session_count, +1) >= session_limit) {
                goto done;
@@ -916,9 +914,13 @@ static void *httpd_helper_thread(void *data)
                if (*c) {
                        *c = '\0';
                }
+       } else {
+               ast_http_error(ser, 400, "Bad Request", "Invalid Request");
+               goto done;
        }
 
        /* process "Request Headers" lines */
+       remaining_headers = MAX_HTTP_REQUEST_HEADERS;
        while (fgets(header_line, sizeof(header_line), ser->f)) {
                char *name, *value;
 
@@ -941,6 +943,11 @@ static void *httpd_helper_thread(void *data)
 
                ast_trim_blanks(name);
 
+               if (!remaining_headers--) {
+                       /* Too many headers. */
+                       ast_http_error(ser, 413, "Request Entity Too Large", "Too many headers");
+                       goto done;
+               }
                if (!headers) {
                        headers = ast_variable_new(name, value, __FILE__);
                        tail = headers;
@@ -948,11 +955,17 @@ static void *httpd_helper_thread(void *data)
                        tail->next = ast_variable_new(name, value, __FILE__);
                        tail = tail->next;
                }
-       }
-
-       if (!*uri) {
-               ast_http_error(ser, 400, "Bad Request", "Invalid Request");
-               goto done;
+               if (!tail) {
+                       /*
+                        * Variable allocation failure.
+                        * Try to make some room.
+                        */
+                       ast_variables_destroy(headers);
+                       headers = NULL;
+
+                       ast_http_error(ser, 500, "Server Error", "Out of memory");
+                       goto done;
+               }
        }
 
        handle_uri(ser, uri, http_method, headers);
@@ -961,9 +974,7 @@ done:
        ast_atomic_fetchadd_int(&session_count, -1);
 
        /* clean up all the header information */
-       if (headers) {
-               ast_variables_destroy(headers);
-       }
+       ast_variables_destroy(headers);
 
        if (ser->f) {
                fclose(ser->f);