[asterisk-commits] mjordan: branch 10 r359707 - in /branches/10: ./ main/utils.c

[SVN commits to the Asterisk project]

Author: mjordan
Date: Thu Mar 15 14:06:09 2012
New Revision: 359707

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=359707
Log:
Fix remotely exploitable stack overflow in HTTP manager

There exists a remotely exploitable stack buffer overflow in HTTP digest
authentication handling in Asterisk.  The particular method in question
is only utilized by HTTP AMI.  When parsing the digest information, the
length of the string is not checked when it is copied into temporary buffers
allocated on the stack.

This patch fixes this behavior by parsing out pre-defined key/value pairs
and avoiding unnecessary copies to the stack.

(closes issue ASTERISK-19542)
Reported by: Russell Bryant
Tested by: Matt Jordan
........

Merged revisions 359706 from http://svn.asterisk.org/svn/asterisk/branches/1.8

Modified:
    branches/10/   (props changed)
    branches/10/main/utils.c

Propchange: branches/10/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.

Modified: branches/10/main/utils.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/main/utils.c?view=diff&rev=359707&r1=359706&r2=359707
==============================================================================
--- branches/10/main/utils.c (original)
+++ branches/10/main/utils.c Thu Mar 15 14:06:09 2012
@@ -1989,9 +1989,28 @@
  * pedantic arg can be set to nonzero if we need to do addition Digest check.
  */
 int ast_parse_digest(const char *digest, struct ast_http_digest *d, int request, int pedantic) {
-	int i;
-	char *c, key[512], val[512];
+	char *c;
 	struct ast_str *str = ast_str_create(16);
+
+	/* table of recognised keywords, and places where they should be copied */
+	const struct x {
+		const char *key;
+		const ast_string_field *field;
+	} *i, keys[] = {
+		{ "username=", &d->username },
+		{ "realm=", &d->realm },
+		{ "nonce=", &d->nonce },
+		{ "uri=", &d->uri },
+		{ "domain=", &d->domain },
+		{ "response=", &d->response },
+		{ "cnonce=", &d->cnonce },
+		{ "opaque=", &d->opaque },
+		/* Special cases that cannot be directly copied */
+		{ "algorithm=", NULL },
+		{ "qop=", NULL },
+		{ "nc=", NULL },
+		{ NULL, 0 },
+	};
 
 	if (ast_strlen_zero(digest) || !d || !str) {
 		ast_free(str);
@@ -2010,72 +2029,55 @@
 	c += strlen("Digest ");
 
 	/* lookup for keys/value pair */
-	while (*c && *(c = ast_skip_blanks(c))) {
+	while (c && *c && *(c = ast_skip_blanks(c))) {
 		/* find key */
-		i = 0;
-		while (*c && *c != '=' && *c != ',' && !isspace(*c)) {
-			key[i++] = *c++;
-		}
-		key[i] = '\0';
-		c = ast_skip_blanks(c);
-		if (*c == '=') {
-			c = ast_skip_blanks(++c);
-			i = 0;
-			if (*c == '\"') {
-				/* in quotes. Skip first and look for last */
-				c++;
-				while (*c && *c != '\"') {
-					if (*c == '\\' && c[1] != '\0') { /* unescape chars */
-						c++;
+		for (i = keys; i->key != NULL; i++) {
+			char *src, *separator;
+			int unescape = 0;
+			if (strncasecmp(c, i->key, strlen(i->key)) != 0) {
+				continue;
+			}
+
+			/* Found. Skip keyword, take text in quotes or up to the separator. */
+			c += strlen(i->key);
+			if (*c == '"') {
+				src = ++c;
+				separator = "\"";
+				unescape = 1;
+			} else {
+				src = c;
+				separator = ",";
+			}
+			strsep(&c, separator); /* clear separator and move ptr */
+			if (unescape) {
+				ast_unescape_c(src);
+			}
+			if (i->field) {
+				ast_string_field_ptr_set(d, i->field, src);
+			} else {
+				/* Special cases that require additional procesing */
+				if (!strcasecmp(i->key, "algorithm=")) {
+					if (strcasecmp(src, "MD5")) {
+						ast_log(LOG_WARNING, "Digest algorithm: \"%s\" not supported.\n", src);
+						ast_free(str);
+						return -1;
 					}
-					val[i++] = *c++;
-				}
-			} else {
-				/* token */
-				while (*c && *c != ',' && !isspace(*c)) {
-					val[i++] = *c++;
+				} else if (!strcasecmp(i->key, "qop=") && !strcasecmp(src, "auth")) {
+					d->qop = 1;
+				} else if (!strcasecmp(i->key, "nc=")) {
+					unsigned long u;
+					if (sscanf(src, "%30lx", &u) != 1) {
+						ast_log(LOG_WARNING, "Incorrect Digest nc value: \"%s\".\n", src);
+						ast_free(str);
+						return -1;
+					}
+					ast_string_field_set(d, nc, src);
 				}
 			}
-			val[i] = '\0';
-		}
-
-		while (*c && *c != ',') {
-			c++;
-		}
-		if (*c) {
-			c++;
-		}
-
-		if (!strcasecmp(key, "username")) {
-			ast_string_field_set(d, username, val);
-		} else if (!strcasecmp(key, "realm")) {
-			ast_string_field_set(d, realm, val);
-		} else if (!strcasecmp(key, "nonce")) {
-			ast_string_field_set(d, nonce, val);
-		} else if (!strcasecmp(key, "uri")) {
-			ast_string_field_set(d, uri, val);
-		} else if (!strcasecmp(key, "domain")) {
-			ast_string_field_set(d, domain, val);
-		} else if (!strcasecmp(key, "response")) {
-			ast_string_field_set(d, response, val);
-		} else if (!strcasecmp(key, "algorithm")) {
-			if (strcasecmp(val, "MD5")) {
-				ast_log(LOG_WARNING, "Digest algorithm: \"%s\" not supported.\n", val);
-				return -1;
-			}
-		} else if (!strcasecmp(key, "cnonce")) {
-			ast_string_field_set(d, cnonce, val);
-		} else if (!strcasecmp(key, "opaque")) {
-			ast_string_field_set(d, opaque, val);
-		} else if (!strcasecmp(key, "qop") && !strcasecmp(val, "auth")) {
-			d->qop = 1;
-		} else if (!strcasecmp(key, "nc")) {
-			unsigned long u;
-			if (sscanf(val, "%30lx", &u) != 1) {
-				ast_log(LOG_WARNING, "Incorrect Digest nc value: \"%s\".\n", val);
-				return -1;
-			}
-			ast_string_field_set(d, nc, val);
+			break;
+		}
+		if (i->key == NULL) { /* not found, try ',' */
+			strsep(&c, ",");
 		}
 	}
 	ast_free(str);