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
........

Merged revisions 359707 from http://svn.asterisk.org/svn/asterisk/branches/10


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@359708 65c4cc65-6c06-0410-ace0-fbb531ad65f3

1 files changed, 65 insertions, 63 deletions

diff --git a/main/utils.c b/main/utils.c
index 2737eda65..256e61f0b 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -1989,10 +1989,29 @@ int ast_utils_init(void)
  * 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);
 		return -1;
@@ -2010,72 +2029,55 @@ int ast_parse_digest(const char *digest, struct ast_http_digest *d, int request,
 	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++;
-					}
-					val[i++] = *c++;
-				}
-			} else {
-				/* token */
-				while (*c && *c != ',' && !isspace(*c)) {
-					val[i++] = *c++;
-				}
+		for (i = keys; i->key != NULL; i++) {
+			char *src, *separator;
+			int unescape = 0;
+			if (strncasecmp(c, i->key, strlen(i->key)) != 0) {
+				continue;
 			}
-			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;
+			/* 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 = ",";
 			}
-		} 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;
+			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;
+					}
+				} 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);
+				}
 			}
-			ast_string_field_set(d, nc, val);
+			break;
+		}
+		if (i->key == NULL) { /* not found, try ',' */
+			strsep(&c, ",");
 		}
 	}
 	ast_free(str);