diff options
author | David M. Lee <dlee@digium.com> | 2013-01-18 05:31:23 +0000 |
---|---|---|
committer | David M. Lee <dlee@digium.com> | 2013-01-18 05:31:23 +0000 |
commit | be727bf0d217ae086f5df3f1a3e3338be7c71a48 (patch) | |
tree | 8396e3281f45a8830b372dc37032938d0b03e8ec /channels | |
parent | ea78b7cbc8e8806f1d55ba30726a8588391ffb00 (diff) |
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
Diffstat (limited to 'channels')
-rw-r--r-- | channels/chan_sip.c | 86 | ||||
-rw-r--r-- | channels/sip/include/reqresp_parser.h | 11 | ||||
-rw-r--r-- | channels/sip/reqresp_parser.c | 53 |
3 files changed, 140 insertions, 10 deletions
diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 692254358..86009f927 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -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); diff --git a/channels/sip/include/reqresp_parser.h b/channels/sip/include/reqresp_parser.h index 9dfec7d39..02b046bc7 100644 --- a/channels/sip/include/reqresp_parser.h +++ b/channels/sip/include/reqresp_parser.h @@ -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 diff --git a/channels/sip/reqresp_parser.c b/channels/sip/reqresp_parser.c index 7893aace5..9ef4fee17 100644 --- a/channels/sip/reqresp_parser.c +++ b/channels/sip/reqresp_parser.c @@ -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; |