[asterisk-commits] rmudgett: branch certified-11.6 r410429 - in /certified/branches/11.6: ./ main/

[SVN commits to the Asterisk project]

Author: rmudgett
Date: Mon Mar 10 12:34:29 2014
New Revision: 410429

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=410429
Log:
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 410380 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 410381 from http://svn.asterisk.org/svn/asterisk/branches/11

Modified:
    certified/branches/11.6/   (props changed)
    certified/branches/11.6/main/http.c

Propchange: certified/branches/11.6/
------------------------------------------------------------------------------
--- branch-11-merged (original)
+++ branch-11-merged Mon Mar 10 12:34:29 2014
@@ -1,1 +1,1 @@
-/branches/11:396884,399513,400075-400093,401446,401960,402450,403863,403917,404344,405215,405380,405431,406341,406418,407727,408388,410311
+/branches/11:396884,399513,400075-400093,401446,401960,402450,403863,403917,404344,405215,405380,405431,406341,406418,407727,408388,410311,410381

Modified: certified/branches/11.6/main/http.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/11.6/main/http.c?view=diff&rev=410429&r1=410428&r2=410429
==============================================================================
--- certified/branches/11.6/main/http.c (original)
+++ certified/branches/11.6/main/http.c Mon Mar 10 12:34:29 2014
@@ -186,9 +186,7 @@
 			break;
 		}
 	}
-	if (cookies) {
-		ast_variables_destroy(cookies);
-	}
+	ast_variables_destroy(cookies);
 	return mngid;
 }
 
@@ -805,12 +803,13 @@
 }*/
 #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;
@@ -840,21 +839,19 @@
 /* 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 (!strcasecmp(v->name, "Cookie")) {
-			char *tmp = ast_strdupa(v->value);
-			if (cookies) {
-				ast_variables_destroy(cookies);
-			}
-
-			cookies = parse_cookies(tmp);
+			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)
 {
@@ -865,6 +862,7 @@
 	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;
@@ -899,9 +897,13 @@
 		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;
 
@@ -924,6 +926,11 @@
 
 		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;
@@ -931,11 +938,17 @@
 			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);
@@ -944,9 +957,7 @@
 	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);