From: Richard Mudgett Date: Thu, 19 Jan 2012 23:25:05 +0000 (+0000) Subject: Misc minor fixes in reqresp_parser.c and chan_sip.c. X-Git-Tag: 10.2.0-rc1~43 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=33dc146b637ed73224b605a0171abbe0bceb5efa;p=thirdparty%2Fasterisk.git Misc minor fixes in reqresp_parser.c and chan_sip.c. * Fix corner cases in get_calleridname() parsing and ensure that the output buffer is nul terminated. * Make get_calleridname() truncate the name it parses if the given buffer is too small rather than abandoning the parse and not returning anything for the name. Adjusted get_calleridname_test() unit test to handle the truncation change. * Fix get_in_brackets_test() unit test to check the results of get_in_brackets() correctly. * Fix parse_name_andor_addr() to not return the address of a local buffer. This function is currently not used. * Fix potential NULL pointer dereference in sip_sendtext(). * No need to memset(calleridname) in check_user_full() or tmp_name in get_name_and_number() because get_calleridname() ensures that it is nul terminated. * Reply with an accurate response if get_msg_text() fails in receive_message(). This is academic in v1.8 because get_msg_text() can never fail. ........ Merged revisions 351618 from http://svn.asterisk.org/svn/asterisk/branches/1.8 git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/10@351646 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 34a72fb4bc..af1689fd41 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -4475,20 +4475,26 @@ static const char *sip_get_callid(struct ast_channel *chan) static int sip_sendtext(struct ast_channel *ast, const char *text) { struct sip_pvt *dialog = ast->tech_pvt; - int debug = sip_debug_test_pvt(dialog); + int debug; - if (!dialog) + if (!dialog) { return -1; + } /* NOT ast_strlen_zero, because a zero-length message is specifically * allowed by RFC 3428 (See section 10, Examples) */ - if (!text) + if (!text) { return 0; + } if(!is_method_allowed(&dialog->allowed_methods, SIP_MESSAGE)) { ast_debug(2, "Trying to send MESSAGE to device that does not support it.\n"); return(0); } - if (debug) + + debug = sip_debug_test_pvt(dialog); + if (debug) { ast_verbose("Sending text %s on %s\n", text, ast->name); + } + transmit_message_with_text(dialog, text, 0, 0); return 0; } @@ -16289,7 +16295,6 @@ static enum check_auth_result check_user_full(struct sip_pvt *p, struct sip_requ ast_copy_string(from, sip_get_header(req, "From"), sizeof(from)); /* XXX here tries to map the username for invite things */ - memset(calleridname, 0, sizeof(calleridname)); /* strip the display-name portion off the beginning of the FROM header. */ if (!(of = (char *) get_calleridname(from, calleridname, sizeof(calleridname)))) { @@ -16509,9 +16514,10 @@ static void receive_message(struct sip_pvt *p, struct sip_request *req, struct a if (get_msg_text2(&buf, req)) { ast_log(LOG_WARNING, "Unable to retrieve text from %s\n", p->callid); - transmit_response(p, "202 Accepted", req); - if (!p->owner) + transmit_response(p, "500 Internal Server Error", req); + if (!p->owner) { sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); + } return; } diff --git a/channels/sip/reqresp_parser.c b/channels/sip/reqresp_parser.c index d135e00733..b646f7bbb5 100644 --- a/channels/sip/reqresp_parser.c +++ b/channels/sip/reqresp_parser.c @@ -685,63 +685,77 @@ const char *get_calleridname(const char *input, char *output, size_t outputsize) char *orig_output = output; const char *orig_input = input; + if (!output || !outputsize) { + /* Bad output parameters. Should never happen. */ + return input; + } + /* clear any empty characters in the beginning */ input = ast_skip_blanks(input); - /* no data at all or no storage room? */ - if (!input || *input == '<' || !outputsize || !output) { - return orig_input; - } - /* make sure the output buffer is initilized */ *orig_output = '\0'; /* make room for '\0' at the end of the output buffer */ - outputsize--; + --outputsize; + + /* no data at all or no display name? */ + if (!input || *input == '<') { + return input; + } /* quoted-string rules */ if (input[0] == '"') { input++; /* skip the first " */ - for (;((outputsize > 0) && *input); input++) { + for (; *input; ++input) { if (*input == '"') { /* end of quoted-string */ break; } else if (*input == 0x5c) { /* quoted-pair = "\" (%x00-09 / %x0B-0C / %x0E-7F) */ - input++; - if (!*input || (unsigned char)*input > 0x7f || *input == 0xa || *input == 0xd) { + ++input; + if (!*input) { + break; + } + if ((unsigned char) *input > 0x7f || *input == 0xa || *input == 0xd) { continue; /* not a valid quoted-pair, so skip it */ } - } else if (((*input != 0x9) && ((unsigned char) *input < 0x20)) || - (*input == 0x7f)) { + } else if ((*input != 0x9 && (unsigned char) *input < 0x20) + || *input == 0x7f) { continue; /* skip this invalid character. */ } - *output++ = *input; - outputsize--; + if (0 < outputsize) { + /* We still have room for the output display-name. */ + *output++ = *input; + --outputsize; + } } /* if this is successful, input should be at the ending quote */ - if (!input || *input != '"') { + if (*input != '"') { ast_log(LOG_WARNING, "No ending quote for display-name was found\n"); *orig_output = '\0'; return orig_input; } /* make sure input is past the last quote */ - input++; + ++input; - /* terminate outbuf */ + /* terminate output */ *output = '\0'; } else { /* either an addr-spec or tokenLWS-combo */ - for (;((outputsize > 0) && *input); input++) { + for (; *input; ++input) { /* token or WSP (without LWS) */ if ((*input >= '0' && *input <= '9') || (*input >= 'A' && *input <= 'Z') || (*input >= 'a' && *input <= 'z') || *input == '-' || *input == '.' || *input == '!' || *input == '%' || *input == '*' || *input == '_' || *input == '+' || *input == '`' || *input == '\'' || *input == '~' || *input == 0x9 || *input == ' ') { - *output++ = *input; - outputsize -= 1; + if (0 < outputsize) { + /* We still have room for the output display-name. */ + *output++ = *input; + --outputsize; + } } else if (*input == '<') { /* end of tokenLWS-combo */ /* we could assert that the previous char is LWS, but we don't care */ break; @@ -759,10 +773,10 @@ const char *get_calleridname(const char *input, char *output, size_t outputsize) return orig_input; } - /* set NULL while trimming trailing whitespace */ + /* terminate output while trimming any trailing whitespace */ do { *output-- = '\0'; - } while (*output == 0x9 || *output == ' '); /* we won't go past orig_output as first was a non-space */ + } while (orig_output <= output && (*output == 0x9 || *output == ' ')); } return input; @@ -771,11 +785,12 @@ const char *get_calleridname(const char *input, char *output, size_t outputsize) AST_TEST_DEFINE(get_calleridname_test) { int res = AST_TEST_PASS; - const char *in1 = "\" quoted-text internal \\\" quote \""; + const char *in1 = " \" quoted-text internal \\\" quote \""; const char *in2 = " token text with no quotes "; const char *overflow1 = " \"quoted-text overflow 1234567890123456789012345678901234567890\" "; + const char *overflow2 = " non-quoted text overflow 1234567890123456789012345678901234567890 "; const char *noendquote = " \"quoted-text no end "; - const char *addrspec = " \"sip:blah@blah "; + const char *addrspec = " sip:blah@blah"; const char *no_quotes_no_brackets = "blah@blah"; const char *after_dname; char dname[40]; @@ -810,11 +825,19 @@ AST_TEST_DEFINE(get_calleridname_test) /* quoted-text buffer overflow */ after_dname = get_calleridname(overflow1, dname, sizeof(dname)); ast_test_status_update(test, "overflow display-name1: %s\nafter: %s\n", dname, after_dname); - if (*dname != '\0' && after_dname != overflow1) { + if (strcmp(dname, "quoted-text overflow 123456789012345678")) { ast_test_status_update(test, "overflow display-name1 test failed\n"); res = AST_TEST_FAIL; } + /* non-quoted-text buffer overflow */ + after_dname = get_calleridname(overflow2, dname, sizeof(dname)); + ast_test_status_update(test, "overflow display-name2: %s\nafter: %s\n", dname, after_dname); + if (strcmp(dname, "non-quoted text overflow 12345678901234")) { + ast_test_status_update(test, "overflow display-name2 test failed\n"); + res = AST_TEST_FAIL; + } + /* quoted-text buffer with no terminating end quote */ after_dname = get_calleridname(noendquote, dname, sizeof(dname)); ast_test_status_update(test, "noendquote display-name1: %s\nafter: %s\n", dname, after_dname); @@ -825,7 +848,7 @@ AST_TEST_DEFINE(get_calleridname_test) /* addr-spec rather than display-name. */ after_dname = get_calleridname(addrspec, dname, sizeof(dname)); - ast_test_status_update(test, "noendquote display-name1: %s\nafter: %s\n", dname, after_dname); + ast_test_status_update(test, "addr-spec display-name1: %s\nafter: %s\n", dname, after_dname); if (*dname != '\0' && after_dname != addrspec) { ast_test_status_update(test, "detection of addr-spec failed\n"); res = AST_TEST_FAIL; @@ -839,14 +862,13 @@ AST_TEST_DEFINE(get_calleridname_test) res = AST_TEST_FAIL; } - return res; } int get_name_and_number(const char *hdr, char **name, char **number) { char header[256]; - char tmp_name[50] = { 0, }; + char tmp_name[50]; char *tmp_number = NULL; char *hostport = NULL; char *dummy = NULL; @@ -1069,14 +1091,14 @@ char *get_in_brackets(char *tmp) AST_TEST_DEFINE(get_in_brackets_test) { int res = AST_TEST_PASS; - char *in_brackets = ""; + char in_brackets[] = "sip:name:secret@host:port;transport=tcp?headers=testblah&headers2=blahblah"; char no_name[] = ""; char quoted_string[] = "\"I'm a quote stri>"; char missing_end_quote[] = "\"I'm a quote string "; char name_no_quotes[] = "name not in quotes "; char no_end_bracket[] = "name not in quotes uri,sizeof(uri)); + ast_copy_string(uri,testdataptr->uri,sizeof(uri)); if (parse_name_andor_addr(uri, "sip:,sips:", testdataptr->nameptr, testdataptr->userptr, @@ -1330,17 +1366,17 @@ AST_TEST_DEFINE(parse_name_andor_addr_test) ((testdataptr->residueptr) && strcmp(testdataptr->residue, residue)) || ((testdataptr->paramsptr) && strcmp(testdataptr->params.transport,params.transport)) || ((testdataptr->paramsptr) && strcmp(testdataptr->params.user,params.user)) - ) { - ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc); - res = AST_TEST_FAIL; + ) { + ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc); + res = AST_TEST_FAIL; } } - return res; } -int get_comma(char *in, char **out) { +int get_comma(char *in, char **out) +{ char *c; char *parse = in; if (out) { @@ -1376,35 +1412,35 @@ int get_comma(char *in, char **out) { return 1; } -int parse_contact_header(char *contactheader, struct contactliststruct *contactlist) { +int parse_contact_header(char *contactheader, struct contactliststruct *contactlist) +{ int res; int last; char *comma; char *residue; char *param; char *value; - struct contact *contact=NULL; + struct contact *split_contact = NULL; if (*contactheader == '*') { return 1; } - contact = malloc(sizeof(*contact)); - - AST_LIST_HEAD_SET_NOLOCK(contactlist, contact); - while ((last = get_comma(contactheader,&comma)) != -1) { + split_contact = ast_calloc(1, sizeof(*split_contact)); + AST_LIST_HEAD_SET_NOLOCK(contactlist, split_contact); + while ((last = get_comma(contactheader, &comma)) != -1) { res = parse_name_andor_addr(contactheader, "sip:,sips:", - &contact->name, &contact->user, - &contact->pass, &contact->hostport, - &contact->params, &contact->headers, - &residue); + &split_contact->name, &split_contact->user, + &split_contact->pass, &split_contact->hostport, + &split_contact->params, &split_contact->headers, + &residue); if (res == -1) { return res; } /* parse contact params */ - contact->expires = contact->q = ""; + split_contact->expires = split_contact->q = ""; while ((value = strchr(residue,'='))) { *value++ = '\0'; @@ -1417,20 +1453,19 @@ int parse_contact_header(char *contactheader, struct contactliststruct *contactl } if (!strcmp(param,"expires")) { - contact->expires = value; + split_contact->expires = value; } else if (!strcmp(param,"q")) { - contact->q = value; + split_contact->q = value; } } - if(last) { + if (last) { return 0; } contactheader = comma; - contact = malloc(sizeof(*contact)); - AST_LIST_INSERT_TAIL(contactlist, contact, list); - + split_contact = ast_calloc(1, sizeof(*split_contact)); + AST_LIST_INSERT_TAIL(contactlist, split_contact, list); } return last; } @@ -1563,16 +1598,15 @@ AST_TEST_DEFINE(parse_contact_header_test) strcmp(tdcontactptr->params.transport, contactptr->params.transport) || strcmp(tdcontactptr->params.ttl, contactptr->params.ttl) || (tdcontactptr->params.lr != contactptr->params.lr) - ) { + ) { ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc); res = AST_TEST_FAIL; break; } - contactptr = AST_LIST_NEXT(contactptr,list); + contactptr = AST_LIST_NEXT(contactptr,list); } } - } return res;