Fix Record-Route parsing for large headers.

Record-Route parsing copied the header into a char[256] array, which can
be a problem if the header is longer than that. This patch parses the
header in place, without the copy, avoiding the issue.

In addition to the original patch, I added a unit test for the new
get_in_brackets_const function.

(closes issue ASTERISK-20837)
Reported by: Corey Farrell
Patches:
	chan_sip-build_route-optimized-rev1.patch uploaded by Corey Farrell (license 5909)
	(with minor changes by dlee)
........

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

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


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@379394 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
David M. Lee
2013-01-18 05:31:23 +00:00
parent ea78b7cbc8
commit be727bf0d2
3 changed files with 140 additions and 10 deletions

View File

@@ -16181,21 +16181,25 @@ static void build_route(struct sip_pvt *p, struct sip_request *req, int backward
/* 1st we pass through all the hops in any Record-Route headers */
for (;;) {
/* Each Record-Route header */
char rr_copy[256];
char *rr_copy_ptr;
char *rr_iter;
int len = 0;
const char *uri;
rr = __get_header(req, "Record-Route", &start);
if (*rr == '\0') {
break;
}
ast_copy_string(rr_copy, rr, sizeof(rr_copy));
rr_copy_ptr = rr_copy;
while ((rr_iter = strsep(&rr_copy_ptr, ","))) { /* Each route entry */
char *uri = get_in_brackets(rr_iter);
len = strlen(uri) + 1;
/* Make a struct route */
while (!get_in_brackets_const(rr, &uri, &len)) {
len++;
rr = strchr(rr, ',');
if(rr >= uri && rr < (uri + len)) {
/* comma inside brackets*/
const char *next_br = strchr(rr, '<');
if (next_br && next_br < (uri + len)) {
rr++;
continue;
}
continue;
}
if ((thishop = ast_malloc(sizeof(*thishop) + len))) {
/* ast_calloc is not needed because all fields are initialized in this block */
ast_copy_string(thishop->hop, uri, len);
ast_debug(2, "build_route: Record-Route hop: <%s>\n", thishop->hop);
/* Link in */
@@ -16218,6 +16222,13 @@ static void build_route(struct sip_pvt *p, struct sip_request *req, int backward
tail = thishop;
}
}
rr = strchr(uri + len, ',');
if (rr == NULL) {
/* No more field-values, we're done with this header */
break;
}
/* Advance past comma */
rr++;
}
}
@@ -34032,6 +34043,59 @@ AST_TEST_DEFINE(test_tcp_message_fragmentation)
return res;
}
AST_TEST_DEFINE(get_in_brackets_const_test)
{
const char *input;
const char *start = NULL;
int len = 0;
int res;
#define CHECK_RESULTS(in, expected_res, expected_start, expected_len) do { \
input = (in); \
res = get_in_brackets_const(input, &start, &len); \
if ((expected_res) != res) { \
ast_test_status_update(test, "Unexpected result: %d != %d\n", expected_res, res); \
return AST_TEST_FAIL; \
} \
if ((expected_start) != start) { \
const char *e = expected_start ? expected_start : "(null)"; \
const char *a = start ? start : "(null)"; \
ast_test_status_update(test, "Unexpected start: %s != %s\n", e, a); \
return AST_TEST_FAIL; \
} \
if ((expected_len) != len) { \
ast_test_status_update(test, "Unexpected len: %d != %d\n", expected_len, len); \
return AST_TEST_FAIL; \
} \
} while(0)
switch (cmd) {
case TEST_INIT:
info->name = __func__;
info->category = "/channels/chan_sip/";
info->summary = "get_in_brackets_const test";
info->description =
"Tests the get_in_brackets_const function";
return AST_TEST_NOT_RUN;
case TEST_EXECUTE:
break;
}
CHECK_RESULTS("", 1, NULL, -1);
CHECK_RESULTS("normal <test>", 0, input + 8, 4);
CHECK_RESULTS("\"normal\" <test>", 0, input + 10, 4);
CHECK_RESULTS("not normal <test", -1, NULL, -1);
CHECK_RESULTS("\"yes < really\" <test>", 0, input + 16, 4);
CHECK_RESULTS("\"even > this\" <test>", 0, input + 15, 4);
CHECK_RESULTS("<sip:id1@10.10.10.10;lr>", 0, input + 1, 22);
CHECK_RESULTS("<sip:id1@10.10.10.10;lr>, <sip:id1@10.10.10.20;lr>", 0, input + 1, 22);
CHECK_RESULTS("<sip:id1,id2@10.10.10.10;lr>", 0, input + 1, 26);
CHECK_RESULTS("<sip:id1@10., <sip:id2@10.10.10.10;lr>", 0, input + 1, 36);
CHECK_RESULTS("\"quoted text\" <sip:dlg1@10.10.10.10;lr>", 0, input + 15, 23);
return AST_TEST_PASS;
}
#endif
#define DATA_EXPORT_SIP_PEER(MEMBER) \
@@ -34291,6 +34355,7 @@ static int load_module(void)
AST_TEST_REGISTER(test_sip_peers_get);
AST_TEST_REGISTER(test_sip_mwi_subscribe_parse);
AST_TEST_REGISTER(test_tcp_message_fragmentation);
AST_TEST_REGISTER(get_in_brackets_const_test);
#endif
/* Register AstData providers */
@@ -34419,6 +34484,7 @@ static int unload_module(void)
AST_TEST_UNREGISTER(test_sip_peers_get);
AST_TEST_UNREGISTER(test_sip_mwi_subscribe_parse);
AST_TEST_UNREGISTER(test_tcp_message_fragmentation);
AST_TEST_UNREGISTER(get_in_brackets_const_test);
#endif
/* Unregister all the AstData providers */
ast_data_unregister(NULL);

View File

@@ -90,6 +90,17 @@ int get_name_and_number(const char *hdr, char **name, char **number);
*/
char *get_in_brackets(char *tmp);
/*! \brief Get text in brackets on a const without copy
*
* \param src String to search
* \param[out] start Set to first character inside left bracket.
* \param[out] length Set to lenght of string inside brackets
* \retval 0 success
* \retval -1 failure
* \retval 1 no brackets so got all
*/
int get_in_brackets_const(const char *src,const char **start,int *length);
/*! \brief Get text in brackets and any trailing residue
*
* \retval 0 success

View File

@@ -948,6 +948,59 @@ AST_TEST_DEFINE(get_name_and_number_test)
return res;
}
int get_in_brackets_const(const char *src,const char **start,int *length)
{
const char *parse = src;
const char *first_bracket;
const char *second_bracket;
if (start == NULL) {
return -1;
}
if (length == NULL) {
return -1;
}
*start = NULL;
*length = -1;
if (ast_strlen_zero(src)) {
return 1;
}
/*
* Skip any quoted text until we find the part in brackets.
* On any error give up and return -1
*/
while ( (first_bracket = strchr(parse, '<')) ) {
const char *first_quote = strchr(parse, '"');
first_bracket++;
if (!first_quote || first_quote >= first_bracket) {
break; /* no need to look at quoted part */
}
/* the bracket is within quotes, so ignore it */
parse = find_closing_quote(first_quote + 1, NULL);
if (!*parse) {
ast_log(LOG_WARNING, "No closing quote found in '%s'\n", src);
return -1;
}
parse++;
}
/* Require a first bracket. Unlike get_in_brackets_full, this procedure is passed a const,
* so it can expect a pointer to an original value */
if (!first_bracket) {
ast_log(LOG_WARNING, "No opening bracket found in '%s'\n", src);
return 1;
}
if ((second_bracket = strchr(first_bracket, '>'))) {
*start = first_bracket;
*length = second_bracket - first_bracket;
return 0;
}
ast_log(LOG_WARNING, "No closing bracket found in '%s'\n", src);
return -1;
}
int get_in_brackets_full(char *tmp,char **out,char **residue)
{
const char *parse = tmp;