Author: kmoore Date: Wed Aug 27 10:31:35 2014 New Revision: 422154 URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=422154 Log: CallerID: Fix parsing of malformed callerid
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/ Review: https://reviewboard.asterisk.org/r/3933/ ........ Merged revisions 422112 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 422113 from http://svn.asterisk.org/svn/asterisk/branches/11 ........ Merged revisions 422114 from http://svn.asterisk.org/svn/asterisk/branches/12 Added: branches/13/tests/test_callerid.c - copied unchanged from r422114, branches/12/tests/test_callerid.c Modified: branches/13/ (props changed) branches/13/channels/chan_sip.c branches/13/include/asterisk/utils.h branches/13/main/callerid.c branches/13/main/utils.c branches/13/res/res_pjsip_caller_id.c branches/13/tests/test_utils.c Propchange: branches/13/ ------------------------------------------------------------------------------ Binary property 'branch-12-merged' - no diff available. Modified: branches/13/channels/chan_sip.c URL: http://svnview.digium.com/svn/asterisk/branches/13/channels/chan_sip.c?view=diff&rev=422154&r1=422153&r2=422154 ============================================================================== --- branches/13/channels/chan_sip.c (original) +++ branches/13/channels/chan_sip.c Wed Aug 27 10:31:35 2014 @@ -12538,6 +12538,7 @@ { 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; @@ -12563,6 +12564,7 @@ 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(&connected_id); if (((lid_pres & AST_PRES_RESTRICTION) != AST_PRES_ALLOWED) && @@ -12586,7 +12588,7 @@ 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"); } @@ -12594,14 +12596,14 @@ /* 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: @@ -18857,7 +18859,10 @@ from = (char *) get_calleridname(from, from_name, sizeof(from_name)); from = get_in_brackets(from); if (from_name[0]) { - res |= ast_msg_set_from(msg, "\"%s\" <%s>", from_name, from); + char from_buf[128]; + + ast_escape_quoted(from_name, from_buf, sizeof(from_buf)); + res |= ast_msg_set_from(msg, "\"%s\" <%s>", from_buf, from); } else { res |= ast_msg_set_from(msg, "<%s>", from); } Modified: branches/13/include/asterisk/utils.h URL: http://svnview.digium.com/svn/asterisk/branches/13/include/asterisk/utils.h?view=diff&rev=422154&r1=422153&r2=422154 ============================================================================== --- branches/13/include/asterisk/utils.h (original) +++ branches/13/include/asterisk/utils.h Wed Aug 27 10:31:35 2014 @@ -317,6 +317,15 @@ * \return a pointer to the escaped string */ 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) { Modified: branches/13/main/callerid.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/callerid.c?view=diff&rev=422154&r1=422153&r2=422154 ============================================================================== --- branches/13/main/callerid.c (original) +++ branches/13/main/callerid.c Wed Aug 27 10:31:35 2014 @@ -1009,50 +1009,39 @@ int ast_callerid_parse(char *instr, char **name, char **location) { - char *ns, *ne, *ls, *le; - - /* Try "name" <location> format or name <location> format */ - if ((ls = strrchr(instr, '<')) && (le = strrchr(ls, '>'))) { - *ls = *le = '\0'; /* location found, trim off the brackets */ + char *ls, *le, *name_start; + + /* 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 */ - 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); - } + + 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; } @@ -1079,14 +1068,18 @@ { 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; } Modified: branches/13/main/utils.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/utils.c?view=diff&rev=422154&r1=422153&r2=422154 ============================================================================== --- branches/13/main/utils.c (original) +++ branches/13/main/utils.c Wed Aug 27 10:31:35 2014 @@ -483,6 +483,29 @@ 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'; +} + int ast_xml_escape(const char *string, char * const outbuf, const size_t buflen) { char *dst = outbuf; Modified: branches/13/res/res_pjsip_caller_id.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_caller_id.c?view=diff&rev=422154&r1=422153&r2=422154 ============================================================================== --- branches/13/res/res_pjsip_caller_id.c (original) +++ branches/13/res/res_pjsip_caller_id.c Wed Aug 27 10:31:35 2014 @@ -404,7 +404,11 @@ id_uri = pjsip_uri_get_uri(id_name_addr->uri); if (id->name.valid) { - pj_strdup2(pool, &id_name_addr->display, id->name.str); + int name_buf_len = strlen(id->name.str) * 2 + 1; + char *name_buf = ast_alloca(name_buf_len); + + ast_escape_quoted(id->name.str, name_buf, name_buf_len); + pj_strdup2(pool, &id_name_addr->display, name_buf); } if (id->number.valid) { @@ -438,7 +442,11 @@ id_uri = pjsip_uri_get_uri(id_name_addr->uri); if (id->name.valid) { - pj_strdup2(tdata->pool, &id_name_addr->display, id->name.str); + int name_buf_len = strlen(id->name.str) * 2 + 1; + char *name_buf = ast_alloca(name_buf_len); + + ast_escape_quoted(id->name.str, name_buf, name_buf_len); + pj_strdup2(tdata->pool, &id_name_addr->display, name_buf); } pj_strdup2(tdata->pool, &id_uri->user, id->number.str); Modified: branches/13/tests/test_utils.c URL: http://svnview.digium.com/svn/asterisk/branches/13/tests/test_utils.c?view=diff&rev=422154&r1=422153&r2=422154 ============================================================================== --- branches/13/tests/test_utils.c (original) +++ branches/13/tests/test_utils.c Wed Aug 27 10:31:35 2014 @@ -546,6 +546,100 @@ } +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); @@ -558,6 +652,8 @@ AST_TEST_UNREGISTER(agi_loaded_test); AST_TEST_UNREGISTER(safe_mkdir_test); AST_TEST_UNREGISTER(crypt_test); + AST_TEST_UNREGISTER(quote_mutation); + AST_TEST_UNREGISTER(quote_unescaping); return 0; } @@ -573,6 +669,8 @@ AST_TEST_REGISTER(agi_loaded_test); AST_TEST_REGISTER(safe_mkdir_test); AST_TEST_REGISTER(crypt_test); + AST_TEST_REGISTER(quote_mutation); + AST_TEST_REGISTER(quote_unescaping); return AST_MODULE_LOAD_SUCCESS; } -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits
