Misc minor fixes in reqresp_parser.c and chan_sip.c.

* Fix corner cases in get_calleridname() parsing and ensure that the
output buffer is nul terminated.

* Make get_calleridname() truncate the name it parses if the given buffer
is too small rather than abandoning the parse and not returning anything
for the name.  Adjusted get_calleridname_test() unit test to handle the
truncation change.

* Fix get_in_brackets_test() unit test to check the results of
get_in_brackets() correctly.

* Fix parse_name_andor_addr() to not return the address of a local buffer.
This function is currently not used.

* Fix potential NULL pointer dereference in sip_sendtext().

* No need to memset(calleridname) in check_user_full() or tmp_name in
get_name_and_number() because get_calleridname() ensures that it is nul
terminated.

* Reply with an accurate response if get_msg_text() fails in
receive_message().  This is academic in v1.8 because get_msg_text() can
never fail.
........

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

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


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

1 files changed, 117 insertions, 83 deletions

diff --git a/channels/sip/reqresp_parser.c b/channels/sip/reqresp_parser.c
index d135e0073..b646f7bbb 100644
--- a/channels/sip/reqresp_parser.c
+++ b/channels/sip/reqresp_parser.c
@@ -685,63 +685,77 @@ const char *get_calleridname(const char *input, char *output, size_t outputsize)
 	char *orig_output = output;
 	const char *orig_input = input;
 
+	if (!output || !outputsize) {
+		/* Bad output parameters.  Should never happen. */
+		return input;
+	}
+
 	/* clear any empty characters in the beginning */
 	input = ast_skip_blanks(input);
 
-	/* no data at all or no storage room? */
-	if (!input || *input == '<' || !outputsize || !output) {
-		return orig_input;
-	}
-
 	/* make sure the output buffer is initilized */
 	*orig_output = '\0';
 
 	/* make room for '\0' at the end of the output buffer */
-	outputsize--;
+	--outputsize;
+
+	/* no data at all or no display name? */
+	if (!input || *input == '<') {
+		return input;
+	}
 
 	/* quoted-string rules */
 	if (input[0] == '"') {
 		input++; /* skip the first " */
 
-		for (;((outputsize > 0) && *input); input++) {
+		for (; *input; ++input) {
 			if (*input == '"') {  /* end of quoted-string */
 				break;
 			} else if (*input == 0x5c) { /* quoted-pair = "\" (%x00-09 / %x0B-0C / %x0E-7F) */
-				input++;
-				if (!*input || (unsigned char)*input > 0x7f || *input == 0xa || *input == 0xd) {
+				++input;
+				if (!*input) {
+					break;
+				}
+				if ((unsigned char) *input > 0x7f || *input == 0xa || *input == 0xd) {
 					continue;  /* not a valid quoted-pair, so skip it */
 				}
-			} else if (((*input != 0x9) && ((unsigned char) *input < 0x20)) ||
-			            (*input == 0x7f)) {
+			} else if ((*input != 0x9 && (unsigned char) *input < 0x20)
+				|| *input == 0x7f) {
 				continue; /* skip this invalid character. */
 			}
 
-			*output++ = *input;
-			outputsize--;
+			if (0 < outputsize) {
+				/* We still have room for the output display-name. */
+				*output++ = *input;
+				--outputsize;
+			}
 		}
 
 		/* if this is successful, input should be at the ending quote */
-		if (!input || *input != '"') {
+		if (*input != '"') {
 			ast_log(LOG_WARNING, "No ending quote for display-name was found\n");
 			*orig_output = '\0';
 			return orig_input;
 		}
 
 		/* make sure input is past the last quote */
-		input++;
+		++input;
 
-		/* terminate outbuf */
+		/* terminate output */
 		*output = '\0';
 	} else {  /* either an addr-spec or tokenLWS-combo */
-		for (;((outputsize > 0) && *input); input++) {
+		for (; *input; ++input) {
 			/* token or WSP (without LWS) */
 			if ((*input >= '0' && *input <= '9') || (*input >= 'A' && *input <= 'Z')
 				|| (*input >= 'a' && *input <= 'z') || *input == '-' || *input == '.'
 				|| *input == '!' || *input == '%' || *input == '*' || *input == '_'
 				|| *input == '+' || *input == '`' || *input == '\'' || *input == '~'
 				|| *input == 0x9 || *input == ' ') {
-				*output++ = *input;
-				outputsize -= 1;
+				if (0 < outputsize) {
+					/* We still have room for the output display-name. */
+					*output++ = *input;
+					--outputsize;
+				}
 			} else if (*input == '<') {   /* end of tokenLWS-combo */
 				/* we could assert that the previous char is LWS, but we don't care */
 				break;
@@ -759,10 +773,10 @@ const char *get_calleridname(const char *input, char *output, size_t outputsize)
 			return orig_input;
 		}
 
-		/* set NULL while trimming trailing whitespace */
+		/* terminate output while trimming any trailing whitespace */
 		do {
 			*output-- = '\0';
-		} while (*output == 0x9 || *output == ' '); /* we won't go past orig_output as first was a non-space */
+		} while (orig_output <= output && (*output == 0x9 || *output == ' '));
 	}
 
 	return input;
@@ -771,11 +785,12 @@ const char *get_calleridname(const char *input, char *output, size_t outputsize)
 AST_TEST_DEFINE(get_calleridname_test)
 {
 	int res = AST_TEST_PASS;
-	const char *in1 = "\" quoted-text internal \\\" quote \"<stuff>";
+	const char *in1 = " \" quoted-text internal \\\" quote \"<stuff>";
 	const char *in2 = " token text with no quotes <stuff>";
 	const char *overflow1 = " \"quoted-text overflow 1234567890123456789012345678901234567890\" <stuff>";
+	const char *overflow2 = " non-quoted text overflow 1234567890123456789012345678901234567890 <stuff>";
 	const char *noendquote = " \"quoted-text no end <stuff>";
-	const char *addrspec = " \"sip:blah@blah <stuff>";
+	const char *addrspec = " sip:blah@blah";
 	const char *no_quotes_no_brackets = "blah@blah";
 	const char *after_dname;
 	char dname[40];
@@ -810,11 +825,19 @@ AST_TEST_DEFINE(get_calleridname_test)
 	/* quoted-text buffer overflow */
 	after_dname = get_calleridname(overflow1, dname, sizeof(dname));
 	ast_test_status_update(test, "overflow display-name1: %s\nafter: %s\n", dname, after_dname);
-	if (*dname != '\0' && after_dname != overflow1) {
+	if (strcmp(dname, "quoted-text overflow 123456789012345678")) {
 		ast_test_status_update(test, "overflow display-name1 test failed\n");
 		res = AST_TEST_FAIL;
 	}
 
+	/* non-quoted-text buffer overflow */
+	after_dname = get_calleridname(overflow2, dname, sizeof(dname));
+	ast_test_status_update(test, "overflow display-name2: %s\nafter: %s\n", dname, after_dname);
+	if (strcmp(dname, "non-quoted text overflow 12345678901234")) {
+		ast_test_status_update(test, "overflow display-name2 test failed\n");
+		res = AST_TEST_FAIL;
+	}
+
 	/* quoted-text buffer with no terminating end quote */
 	after_dname = get_calleridname(noendquote, dname, sizeof(dname));
 	ast_test_status_update(test, "noendquote display-name1: %s\nafter: %s\n", dname, after_dname);
@@ -825,7 +848,7 @@ AST_TEST_DEFINE(get_calleridname_test)
 
 	/* addr-spec rather than display-name. */
 	after_dname = get_calleridname(addrspec, dname, sizeof(dname));
-	ast_test_status_update(test, "noendquote display-name1: %s\nafter: %s\n", dname, after_dname);
+	ast_test_status_update(test, "addr-spec display-name1: %s\nafter: %s\n", dname, after_dname);
 	if (*dname != '\0' && after_dname != addrspec) {
 		ast_test_status_update(test, "detection of addr-spec failed\n");
 		res = AST_TEST_FAIL;
@@ -839,14 +862,13 @@ AST_TEST_DEFINE(get_calleridname_test)
 		res = AST_TEST_FAIL;
 	}
 
-
 	return res;
 }
 
 int get_name_and_number(const char *hdr, char **name, char **number)
 {
 	char header[256];
-	char tmp_name[50] = { 0, };
+	char tmp_name[50];
 	char *tmp_number = NULL;
 	char *hostport = NULL;
 	char *dummy = NULL;
@@ -1069,14 +1091,14 @@ char *get_in_brackets(char *tmp)
 AST_TEST_DEFINE(get_in_brackets_test)
 {
 	int res = AST_TEST_PASS;
-	char *in_brackets = "<sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah>";
+	char in_brackets[] = "sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah";
 	char no_name[] = "<sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah>";
 	char quoted_string[] = "\"I'm a quote stri><ng\" <sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah>";
 	char missing_end_quote[] = "\"I'm a quote string <sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah>";
 	char name_no_quotes[] = "name not in quotes <sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah>";
 	char no_end_bracket[] = "name not in quotes <sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah";
 	char no_name_no_brackets[] = "sip:name@host";
-	char missing_start_bracket[] = "name not in quotes sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah>";
+	char missing_start_bracket[] = "sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah>";
 	char *uri = NULL;
 
 	switch (cmd) {
@@ -1093,57 +1115,49 @@ AST_TEST_DEFINE(get_in_brackets_test)
 	}
 
 	/* Test 1, simple get in brackets */
-	if (!(uri = get_in_brackets(no_name)) || !(strcmp(uri, in_brackets))) {
-
-		ast_test_status_update(test, "Test 1, simple get in brackets failed.\n");
+	if (!(uri = get_in_brackets(no_name)) || strcmp(uri, in_brackets)) {
+		ast_test_status_update(test, "Test 1, simple get in brackets failed. %s\n", uri);
 		res = AST_TEST_FAIL;
 	}
 
 	/* Test 2, starts with quoted string */
-	if (!(uri = get_in_brackets(quoted_string)) || !(strcmp(uri, in_brackets))) {
-
-		ast_test_status_update(test, "Test 2, get in brackets with quoted string in front failed.\n");
+	if (!(uri = get_in_brackets(quoted_string)) || strcmp(uri, in_brackets)) {
+		ast_test_status_update(test, "Test 2, get in brackets with quoted string in front failed. %s\n", uri);
 		res = AST_TEST_FAIL;
 	}
 
 	/* Test 3, missing end quote */
-	if (!(uri = get_in_brackets(missing_end_quote)) || !(strcmp(uri, in_brackets))) {
-
-		ast_test_status_update(test, "Test 3, missing end quote failed.\n");
+	if (!(uri = get_in_brackets(missing_end_quote)) || !strcmp(uri, in_brackets)) {
+		ast_test_status_update(test, "Test 3, missing end quote failed. %s\n", uri);
 		res = AST_TEST_FAIL;
 	}
 
 	/* Test 4, starts with a name not in quotes */
-	if (!(uri = get_in_brackets(name_no_quotes)) || !(strcmp(uri, in_brackets))) {
-
-		ast_test_status_update(test, "Test 4, passing name not in quotes failed.\n");
+	if (!(uri = get_in_brackets(name_no_quotes)) || strcmp(uri, in_brackets)) {
+		ast_test_status_update(test, "Test 4, passing name not in quotes failed. %s\n", uri);
 		res = AST_TEST_FAIL;
 	}
 
 	/* Test 5, no end bracket, should just return everything after the first '<'  */
-	if (!(uri = get_in_brackets(no_end_bracket)) || !(strcmp(uri, in_brackets))) {
-
-		ast_test_status_update(test, "Test 5, no end bracket failed.\n");
+	if (!(uri = get_in_brackets(no_end_bracket)) || !strcmp(uri, in_brackets)) {
+		ast_test_status_update(test, "Test 5, no end bracket failed. %s\n", uri);
 		res = AST_TEST_FAIL;
 	}
 
 	/* Test 6, NULL input  */
 	if ((uri = get_in_brackets(NULL))) {
-
 		ast_test_status_update(test, "Test 6, NULL input failed.\n");
 		res = AST_TEST_FAIL;
 	}
 
 	/* Test 7, no name, and no brackets. */
-	if (!(uri = get_in_brackets(no_name_no_brackets)) || (strcmp(uri, "sip:name@host"))) {
-
+	if (!(uri = get_in_brackets(no_name_no_brackets)) || strcmp(uri, "sip:name@host")) {
 		ast_test_status_update(test, "Test 7 failed. %s\n", uri);
 		res = AST_TEST_FAIL;
 	}
 
 	/* Test 8, no start bracket, but with ending bracket. */
-	if (!(uri = get_in_brackets(missing_start_bracket)) || !(strcmp(uri, in_brackets))) {
-
+	if (!(uri = get_in_brackets(missing_start_bracket)) || strcmp(uri, in_brackets)) {
 		ast_test_status_update(test, "Test 8 failed. %s\n", uri);
 		res = AST_TEST_FAIL;
 	}
@@ -1158,20 +1172,42 @@ int parse_name_andor_addr(char *uri, const char *scheme, char **name,
 			  char **residue)
 {
 	char buf[1024];
-	char **residue2=residue;
+	char **residue2 = residue;
+	char *orig_uri = uri;
 	int ret;
+
+	buf[0] = '\0';
 	if (name) {
-		get_calleridname(uri,buf,sizeof(buf));
-		*name = buf;
+		uri = (char *) get_calleridname(uri, buf, sizeof(buf));
 	}
-	ret = get_in_brackets_full(uri,&uri,residue);
-	if (ret == 0) { /* uri is in brackets so do not treat unknown trailing uri parameters as potential messageheader parameters */
-		*residue = *residue + 1; /* step over the first semicolon so as per parse uri residue */
+	ret = get_in_brackets_full(uri, &uri, residue);
+	if (ret == 0) {
+		/*
+		 * The uri is in brackets so do not treat unknown trailing uri
+		 * parameters as potential message header parameters.
+		 */
+		if (residue && **residue) {
+			/* step over the first semicolon as per parse_uri_full residue */
+			*residue = *residue + 1;
+		}
 		residue2 = NULL;
 	}
 
-	return parse_uri_full(uri, scheme, user, pass, hostport, params, headers,
-			      residue2);
+	if (name) {
+		if (buf[0]) {
+			/*
+			 * There is always room at orig_uri for the display-name because
+			 * at least one character has always been removed.  A '"' or '<'
+			 * has been removed.
+			 */
+			strcpy(orig_uri, buf);
+			*name = orig_uri;
+		} else {
+			*name = "";
+		}
+	}
+
+	return parse_uri_full(uri, scheme, user, pass, hostport, params, headers, residue2);
 }
 
 AST_TEST_DEFINE(parse_name_andor_addr_test)
@@ -1313,7 +1349,7 @@ AST_TEST_DEFINE(parse_name_andor_addr_test)
 		name = user = pass = hostport = headers = residue = NULL;
 		params.transport = params.user = params.method = params.ttl = params.maddr = NULL;
 		params.lr = 0;
-	ast_copy_string(uri,testdataptr->uri,sizeof(uri));
+		ast_copy_string(uri,testdataptr->uri,sizeof(uri));
 		if (parse_name_andor_addr(uri, "sip:,sips:",
 					  testdataptr->nameptr,
 					  testdataptr->userptr,
@@ -1330,17 +1366,17 @@ AST_TEST_DEFINE(parse_name_andor_addr_test)
 			((testdataptr->residueptr) && strcmp(testdataptr->residue, residue)) ||
 			((testdataptr->paramsptr) && strcmp(testdataptr->params.transport,params.transport)) ||
 			((testdataptr->paramsptr) && strcmp(testdataptr->params.user,params.user))
-		) {
-				ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc);
-				res = AST_TEST_FAIL;
+			) {
+			ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc);
+			res = AST_TEST_FAIL;
 		}
 	}
 
-
 	return res;
 }
 
-int get_comma(char *in, char **out) {
+int get_comma(char *in, char **out)
+{
 	char *c;
 	char *parse = in;
 	if (out) {
@@ -1376,35 +1412,35 @@ int get_comma(char *in, char **out) {
 	return 1;
 }
 
-int parse_contact_header(char *contactheader, struct contactliststruct *contactlist) {
+int parse_contact_header(char *contactheader, struct contactliststruct *contactlist)
+{
 	int res;
 	int last;
 	char *comma;
 	char *residue;
 	char *param;
 	char *value;
-	struct contact *contact=NULL;
+	struct contact *split_contact = NULL;
 
 	if (*contactheader == '*') {
 		return 1;
 	}
 
-	contact = malloc(sizeof(*contact));
-
-	AST_LIST_HEAD_SET_NOLOCK(contactlist, contact);
-	while ((last = get_comma(contactheader,&comma)) != -1) {
+	split_contact = ast_calloc(1, sizeof(*split_contact));
 
+	AST_LIST_HEAD_SET_NOLOCK(contactlist, split_contact);
+	while ((last = get_comma(contactheader, &comma)) != -1) {
 		res = parse_name_andor_addr(contactheader, "sip:,sips:",
-					    &contact->name, &contact->user,
-					    &contact->pass, &contact->hostport,
-					    &contact->params, &contact->headers,
-					    &residue);
+			&split_contact->name, &split_contact->user,
+			&split_contact->pass, &split_contact->hostport,
+			&split_contact->params, &split_contact->headers,
+			&residue);
 		if (res == -1) {
 			return res;
 		}
 
 		/* parse contact params */
-		contact->expires = contact->q = "";
+		split_contact->expires = split_contact->q = "";
 
 		while ((value = strchr(residue,'='))) {
 			*value++ = '\0';
@@ -1417,20 +1453,19 @@ int parse_contact_header(char *contactheader, struct contactliststruct *contactl
 			}
 
 			if (!strcmp(param,"expires")) {
-				contact->expires = value;
+				split_contact->expires = value;
 			} else if (!strcmp(param,"q")) {
-				contact->q = value;
+				split_contact->q = value;
 			}
 		}
 
-		if(last) {
+		if (last) {
 			return 0;
 		}
 		contactheader = comma;
 
-		contact = malloc(sizeof(*contact));
-		AST_LIST_INSERT_TAIL(contactlist, contact, list);
-
+		split_contact = ast_calloc(1, sizeof(*split_contact));
+		AST_LIST_INSERT_TAIL(contactlist, split_contact, list);
 	}
 	return last;
 }
@@ -1563,16 +1598,15 @@ AST_TEST_DEFINE(parse_contact_header_test)
 					strcmp(tdcontactptr->params.transport, contactptr->params.transport) ||
 					strcmp(tdcontactptr->params.ttl, contactptr->params.ttl) ||
 					(tdcontactptr->params.lr != contactptr->params.lr)
-				) {
+					) {
 					ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc);
 					res = AST_TEST_FAIL;
 					break;
 				}
 
-			contactptr = AST_LIST_NEXT(contactptr,list);
+				contactptr = AST_LIST_NEXT(contactptr,list);
 			}
 		}
-
 	}
 
 	return res;