Fix Record-Route parsing for large headers.
authorDavid M. Lee <dlee@digium.com>
Fri, 18 Jan 2013 05:31:23 +0000 (05:31 +0000)
committerDavid M. Lee <dlee@digium.com>
Fri, 18 Jan 2013 05:31:23 +0000 (05:31 +0000)
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

channels/chan_sip.c
channels/sip/include/reqresp_parser.h
channels/sip/reqresp_parser.c

index 6922543..86009f9 100644 (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);
index 9dfec7d..02b046b 100644 (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
index 7893aac..9ef4fee 100644 (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;