]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
CallerID: Fix parsing of malformed callerid
authorKinsey Moore <kmoore@digium.com>
Wed, 27 Aug 2014 14:25:34 +0000 (14:25 +0000)
committerKinsey Moore <kmoore@digium.com>
Wed, 27 Aug 2014 14:25:34 +0000 (14:25 +0000)
This allows the callerid parsing function to handle malformed input
strings and strings containing escaped and unescaped double quotes.
This also adds a unittest to cover many of the cases where the parsing
algorithm previously failed.

Review: https://reviewboard.asterisk.org/r/3923/

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

channels/chan_sip.c
include/asterisk/utils.h
main/callerid.c
main/utils.c
tests/test_callerid.c [new file with mode: 0644]
tests/test_utils.c

index 933e33a218c1d0f161876a79c5186b2e9b909293..a8e7eb277ba3ade1563ed7747e88ffdb9e45fdc1 100644 (file)
@@ -11251,6 +11251,7 @@ static int add_rpid(struct sip_request *req, struct sip_pvt *p)
 {
        struct ast_str *tmp = ast_str_alloca(256);
        char tmp2[256];
+       char lid_name_buf[128];
        char *lid_num;
        char *lid_name;
        int lid_pres;
@@ -11278,6 +11279,7 @@ static int add_rpid(struct sip_request *req, struct sip_pvt *p)
        if (!lid_name) {
                lid_name = lid_num;
        }
+       ast_escape_quoted(lid_name, lid_name_buf, sizeof(lid_name_buf));
        lid_pres = ast_party_id_presentation(&p->owner->connected.id);
 
        if (((lid_pres & AST_PRES_RESTRICTION) != AST_PRES_ALLOWED) &&
@@ -11301,7 +11303,7 @@ static int add_rpid(struct sip_request *req, struct sip_pvt *p)
                if (ast_test_flag(&p->flags[1], SIP_PAGE2_TRUST_ID_OUTBOUND) != SIP_PAGE2_TRUST_ID_OUTBOUND_LEGACY) {
                        /* trust_id_outbound = yes - Always give full information even if it's private, but append a privacy header
                         * When private data is included */
-                       ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>", lid_name, lid_num, fromdomain);
+                       ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>", lid_name_buf, lid_num, fromdomain);
                        if ((lid_pres & AST_PRES_RESTRICTION) != AST_PRES_ALLOWED) {
                                add_header(req, "Privacy", "id");
                        }
@@ -11309,14 +11311,14 @@ static int add_rpid(struct sip_request *req, struct sip_pvt *p)
                        /* trust_id_outbound = legacy - behave in a non RFC-3325 compliant manner and send anonymized data when
                         * when handling private data. */
                        if ((lid_pres & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED) {
-                               ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>", lid_name, lid_num, fromdomain);
+                               ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>", lid_name_buf, lid_num, fromdomain);
                        } else {
                                ast_str_set(&tmp, -1, "%s", anonymous_string);
                        }
                }
                add_header(req, "P-Asserted-Identity", ast_str_buffer(tmp));
        } else {
-               ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>;party=%s", lid_name, lid_num, fromdomain, p->outgoing_call ? "calling" : "called");
+               ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>;party=%s", lid_name_buf, lid_num, fromdomain, p->outgoing_call ? "calling" : "called");
 
                switch (lid_pres) {
                case AST_PRES_ALLOWED_USER_NUMBER_NOT_SCREENED:
@@ -12550,8 +12552,11 @@ static void add_diversion_header(struct sip_request *req, struct sip_pvt *pvt)
                snprintf(header_text, sizeof(header_text), "<sip:%s@%s>;reason=%s", diverting_number,
                                ast_sockaddr_stringify_host_remote(&pvt->ourip), reason);
        } else {
+               char diverting_name_buf[128];
+
+               ast_escape_quoted(diverting_name, diverting_name_buf, sizeof(diverting_name_buf));
                snprintf(header_text, sizeof(header_text), "\"%s\" <sip:%s@%s>;reason=%s",
-                               diverting_name, diverting_number,
+                               diverting_name_buf, diverting_number,
                                ast_sockaddr_stringify_host_remote(&pvt->ourip), reason);
        }
 
index a6e3a2a7eef499498d456ffd1a302f708c608302..14ec6d70031b2628da59f1ffe99eb770b2f3fa25 100644 (file)
@@ -300,6 +300,15 @@ int ast_xml_escape(const char *string, char *outbuf, size_t buflen);
  */
 char *ast_escape_quoted(const char *string, char *outbuf, int buflen);
 
+/*!
+ * \brief Unescape quotes in a string
+ *
+ * \param quote_str The string with quotes to be unescaped
+ *
+ * \note This function mutates the passed-in string.
+ */
+void ast_unescape_quoted(char *quote_str);
+
 static force_inline void ast_slinear_saturated_add(short *input, short *value)
 {
        int res;
index cb6e58857369d80a62080328d49afe5610948508..3b19932736416f187aa86c41b538bdc3a74a27ab 100644 (file)
@@ -1008,50 +1008,39 @@ int ast_is_shrinkable_phonenumber(const char *exten)
 
 int ast_callerid_parse(char *instr, char **name, char **location)
 {
-       char *ns, *ne, *ls, *le;
+       char *ls, *le, *name_start;
 
-       /* Try "name" <location> format or name <location> format */
-       if ((ls = strrchr(instr, '<')) && (le = strrchr(ls, '>'))) {
-               *ls = *le = '\0';       /* location found, trim off the brackets */
-               *location = ls + 1;     /* and this is the result */
-               if ((ns = strchr(instr, '"')) && (ne = strchr(ns + 1, '"'))) {
-                       *ns = *ne = '\0';       /* trim off the quotes */
-                       *name = ns + 1;         /* and this is the name */
-               } else if (ns) {
-                       /* An opening quote was found but no closing quote was. The closing
-                        * quote may actually be after the end of the bracketed number
-                        */
-                       if (strchr(le + 1, '\"')) {
-                               *ns = '\0';
-                               *name = ns + 1;
-                               ast_trim_blanks(*name);
-                       } else {
-                               *name = NULL;
-                       }
-               } else { /* no quotes, trim off leading and trailing spaces */
-                       *name = ast_skip_blanks(instr);
-                       ast_trim_blanks(*name);
+       /* Handle surrounding quotes */
+       instr = ast_strip_quoted(instr, "\"", "\"");
+
+       /* Try "name" <location> format or name <location> format or with a missing > */
+       if ((ls = strrchr(instr, '<'))) {
+               if ((le = strrchr(ls, '>'))) {
+                       *le = '\0';     /* location found, trim off the brackets */
                }
+               *ls = '\0';
+               *location = ls + 1;     /* and this is the result */
+
+               name_start = ast_strip_quoted(instr, "\"", "\"");
        } else {        /* no valid brackets */
                char tmp[256];
 
                ast_copy_string(tmp, instr, sizeof(tmp));
                ast_shrink_phone_number(tmp);
                if (ast_isphonenumber(tmp)) {   /* Assume it's just a location */
-                       *name = NULL;
+                       name_start = NULL;
                        strcpy(instr, tmp); /* safe, because tmp will always be the same size or smaller than instr */
                        *location = instr;
                } else { /* Assume it's just a name. */
                        *location = NULL;
-                       if ((ns = strchr(instr, '"')) && (ne = strchr(ns + 1, '"'))) {
-                               *ns = *ne = '\0';       /* trim off the quotes */
-                               *name = ns + 1;         /* and this is the name */
-                       } else { /* no quotes, trim off leading and trailing spaces */
-                               *name = ast_skip_blanks(instr);
-                               ast_trim_blanks(*name);
-                       }
+                       name_start = ast_strip_quoted(instr, "\"", "\"");
                }
        }
+
+       if (name_start) {
+               ast_unescape_quoted(name_start);
+       }
+       *name = name_start;
        return 0;
 }
 
@@ -1078,14 +1067,18 @@ char *ast_callerid_merge(char *buf, int bufsiz, const char *name, const char *nu
 {
        if (!unknown)
                unknown = "<unknown>";
-       if (name && num)
-               snprintf(buf, bufsiz, "\"%s\" <%s>", name, num);
-       else if (name) 
+       if (name && num) {
+               char name_buf[128];
+
+               ast_escape_quoted(name, name_buf, sizeof(name_buf));
+               snprintf(buf, bufsiz, "\"%s\" <%s>", name_buf, num);
+       } else if (name) {
                ast_copy_string(buf, name, bufsiz);
-       else if (num)
+       } else if (num) {
                ast_copy_string(buf, num, bufsiz);
-       else
+       } else {
                ast_copy_string(buf, unknown, bufsiz);
+       }
        return buf;
 }
 
index 1bb6e76a4640b1828913916f4ffda6cfc6a75ebe..78c28a6c0235dbd0d425a0152d7b07819d0757e8 100644 (file)
@@ -458,6 +458,28 @@ char *ast_escape_quoted(const char *string, char *outbuf, int buflen)
        return outbuf;
 }
 
+void ast_unescape_quoted(char *quote_str)
+{
+       int esc_pos;
+       int unesc_pos;
+       int quote_str_len = strlen(quote_str);
+
+       for (esc_pos = 0, unesc_pos = 0;
+               esc_pos < quote_str_len;
+               esc_pos++, unesc_pos++) {
+               if (quote_str[esc_pos] == '\\') {
+                       /* at least one more char and current is \\ */
+                       esc_pos++;
+                       if (esc_pos >= quote_str_len) {
+                               break;
+                       }
+               }
+
+               quote_str[unesc_pos] = quote_str[esc_pos];
+       }
+       quote_str[unesc_pos] = '\0';
+}
+
 /*! \brief  ast_uri_decode: Decode SIP URI, URN, URL (overwrite the string)  */
 void ast_uri_decode(char *s) 
 {
diff --git a/tests/test_callerid.c b/tests/test_callerid.c
new file mode 100644 (file)
index 0000000..43b760d
--- /dev/null
@@ -0,0 +1,165 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2014, Kinsey Moore
+ *
+ * Kinsey Moore <kmoore@digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*!
+ * \file
+ * \brief Callerid Tests
+ *
+ * \author\verbatim Kinsey Moore <kmoore@digium.com> \endverbatim
+ *
+ * This is an Asterisk test module for callerid functionality
+ * \ingroup tests
+ */
+
+/*** MODULEINFO
+       <depend>TEST_FRAMEWORK</depend>
+       <support_level>core</support_level>
+ ***/
+
+#include "asterisk.h"
+#include "asterisk/callerid.h"
+
+ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
+
+#include "asterisk/utils.h"
+#include "asterisk/module.h"
+#include "asterisk/test.h"
+
+struct cid_set {
+       char *cid;
+       char *name;
+       char *number;
+};
+
+AST_TEST_DEFINE(parse_nominal)
+{
+       static const struct cid_set cid_sets[] = {
+               {"\"name\" <number>", "name", "number"},
+               {"\"   name  \" <number>", "   name  ", "number"},
+               {"name <number>", "name", "number"},
+               {"         name     <number>", "name", "number"},
+               {"\"\" <number>", NULL, "number"},
+               {"<number>", NULL, "number"},
+               {"name", "name", NULL},
+               {"\"name\"", "name", NULL},
+               {"\"name\" <>", "name", NULL},
+               {"name <>", "name", NULL},
+               {"1234", NULL, "1234"},
+               {"\"na\\\"me\" <number>", "na\"me", "number"},
+       };
+       char *name;
+       char *number;
+       int i;
+
+       switch (cmd) {
+       case TEST_INIT:
+               info->name = "parse_nominal";
+               info->category = "/main/callerid/";
+               info->summary = "Callerid nominal parse unit test";
+               info->description =
+                       "This tests parsing of nominal callerid strings.";
+               return AST_TEST_NOT_RUN;
+       case TEST_EXECUTE:
+               break;
+       }
+
+       for (i = 0; i < ARRAY_LEN(cid_sets); i++) {
+               RAII_VAR(char *, callerid, ast_strdup(cid_sets[i].cid), ast_free);
+
+               ast_callerid_parse(callerid, &name, &number);
+               if (!cid_sets[i].name == !ast_strlen_zero(name) || (cid_sets[i].name && strcmp(name, cid_sets[i].name))) {
+                       ast_test_status_update(test,
+                               "Expected callerid name '%s' instead of '%s'\n",
+                               cid_sets[i].name, name);
+                       return AST_TEST_FAIL;
+               }
+
+               if (!cid_sets[i].number == !ast_strlen_zero(number) || (cid_sets[i].number && strcmp(number, cid_sets[i].number))) {
+                       ast_test_status_update(test,
+                               "Expected callerid number '%s' instead of '%s'\n",
+                               cid_sets[i].number, number);
+                       return AST_TEST_FAIL;
+               }
+       }
+
+       return AST_TEST_PASS;
+}
+
+AST_TEST_DEFINE(parse_off_nominal)
+{
+       static const struct cid_set cid_sets[] = {
+               {"na\\\"me <number>", "na\"me", "number"},
+               {"\"na\"me\" <number>", "na\"me", "number"},
+               {"na\"me <number>", "na\"me", "number"},
+               {"\"name <number>", "\"name", "number"},
+               {"name <number", "name", "number"},
+               {"\"name <number>\"", "name", "number"},
+       };
+       char *name;
+       char *number;
+       int i;
+
+       switch (cmd) {
+       case TEST_INIT:
+               info->name = "parse_off_nominal";
+               info->category = "/main/callerid/";
+               info->summary = "Callerid off-nominal parse unit test";
+               info->description =
+                       "This tests parsing of off-nominal callerid strings.";
+               return AST_TEST_NOT_RUN;
+       case TEST_EXECUTE:
+               break;
+       }
+
+       for (i = 0; i < ARRAY_LEN(cid_sets); i++) {
+               RAII_VAR(char *, callerid, ast_strdup(cid_sets[i].cid), ast_free);
+
+               ast_callerid_parse(callerid, &name, &number);
+               if (!cid_sets[i].name == !ast_strlen_zero(name) || (cid_sets[i].name && strcmp(name, cid_sets[i].name))) {
+                       ast_test_status_update(test,
+                               "Expected callerid name '%s' instead of '%s'\n",
+                               cid_sets[i].name, name);
+                       return AST_TEST_FAIL;
+               }
+
+               if (!cid_sets[i].number == !ast_strlen_zero(number) || (cid_sets[i].number && strcmp(number, cid_sets[i].number))) {
+                       ast_test_status_update(test,
+                               "Expected callerid number '%s' instead of '%s'\n",
+                               cid_sets[i].number, number);
+                       return AST_TEST_FAIL;
+               }
+       }
+
+       return AST_TEST_PASS;
+}
+
+static int unload_module(void)
+{
+       AST_TEST_UNREGISTER(parse_nominal);
+       AST_TEST_UNREGISTER(parse_off_nominal);
+       return 0;
+}
+
+static int load_module(void)
+{
+       AST_TEST_REGISTER(parse_nominal);
+       AST_TEST_REGISTER(parse_off_nominal);
+       return AST_MODULE_LOAD_SUCCESS;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Callerid Parse Tests");
index 5d03a480be8d76a78591c145d581cb075e69a890..c12002de59c523b0aea520b3a39e835da103bd05 100644 (file)
@@ -394,6 +394,100 @@ AST_TEST_DEFINE(agi_loaded_test)
        return res;
 }
 
+struct quote_set {
+       char *input;
+       char *output;
+};
+
+AST_TEST_DEFINE(quote_mutation)
+{
+       char escaped[64];
+       static const struct quote_set escape_sets[] = {
+               {"\"string\"", "\\\"string\\\""},
+               {"\"string", "\\\"string"},
+               {"string\"", "string\\\""},
+               {"string", "string"},
+               {"str\"ing", "str\\\"ing"},
+               {"\"", "\\\""},
+               {"\\\"", "\\\\\\\""},
+       };
+       int i;
+
+       switch (cmd) {
+       case TEST_INIT:
+               info->name = "quote_mutation";
+               info->category = "/main/utils/";
+               info->summary = "Test mutation of quotes in strings";
+               info->description =
+                       "This tests escaping and unescaping of quotes in strings to "
+                       "verify that the original string is recovered.";
+               return AST_TEST_NOT_RUN;
+       case TEST_EXECUTE:
+               break;
+       }
+
+       for (i = 0; i < ARRAY_LEN(escape_sets); i++) {
+               ast_escape_quoted(escape_sets[i].input, escaped, sizeof(escaped));
+
+               if (strcmp(escaped, escape_sets[i].output)) {
+                       ast_test_status_update(test,
+                               "Expected escaped string '%s' instead of '%s'\n",
+                               escape_sets[i].output, escaped);
+                       return AST_TEST_FAIL;
+               }
+
+               ast_unescape_quoted(escaped);
+               if (strcmp(escaped, escape_sets[i].input)) {
+                       ast_test_status_update(test,
+                               "Expected unescaped string '%s' instead of '%s'\n",
+                               escape_sets[i].input, escaped);
+                       return AST_TEST_FAIL;
+               }
+       }
+
+       return AST_TEST_PASS;
+}
+
+AST_TEST_DEFINE(quote_unescaping)
+{
+       static const struct quote_set escape_sets[] = {
+               {"\"string\"", "\"string\""},
+               {"\\\"string\"", "\"string\""},
+               {"\"string\\\"", "\"string\""},
+               {"str\\ing", "string"},
+               {"string\\", "string"},
+               {"\\string", "string"},
+       };
+       int i;
+
+       switch (cmd) {
+       case TEST_INIT:
+               info->name = "quote_unescaping";
+               info->category = "/main/utils/";
+               info->summary = "Test unescaping of off-nominal strings";
+               info->description =
+                       "This tests unescaping of strings which contain a mix of "
+                       "escaped and unescaped sequences.";
+               return AST_TEST_NOT_RUN;
+       case TEST_EXECUTE:
+               break;
+       }
+
+       for (i = 0; i < ARRAY_LEN(escape_sets); i++) {
+               RAII_VAR(char *, escaped, ast_strdup(escape_sets[i].input), ast_free);
+
+               ast_unescape_quoted(escaped);
+               if (strcmp(escaped, escape_sets[i].output)) {
+                       ast_test_status_update(test,
+                               "Expected unescaped string '%s' instead of '%s'\n",
+                               escape_sets[i].output, escaped);
+                       return AST_TEST_FAIL;
+               }
+       }
+
+       return AST_TEST_PASS;
+}
+
 static int unload_module(void)
 {
        AST_TEST_UNREGISTER(uri_encode_decode_test);
@@ -404,6 +498,8 @@ static int unload_module(void)
        AST_TEST_UNREGISTER(crypto_loaded_test);
        AST_TEST_UNREGISTER(adsi_loaded_test);
        AST_TEST_UNREGISTER(agi_loaded_test);
+       AST_TEST_UNREGISTER(quote_mutation);
+       AST_TEST_UNREGISTER(quote_unescaping);
        return 0;
 }
 
@@ -417,6 +513,8 @@ static int load_module(void)
        AST_TEST_REGISTER(crypto_loaded_test);
        AST_TEST_REGISTER(adsi_loaded_test);
        AST_TEST_REGISTER(agi_loaded_test);
+       AST_TEST_REGISTER(quote_mutation);
+       AST_TEST_REGISTER(quote_unescaping);
        return AST_MODULE_LOAD_SUCCESS;
 }