Summary: | ASTERISK-26897: chan_sip: Security vulnerability with client code header | ||||
Reporter: | Alex VillacĂs Lasso (a_villacis) | Labels: | |||
Date Opened: | 2017-03-27 05:45:30 | Date Closed: | 2017-04-04 06:23:32 | ||
Priority: | Major | Regression? | |||
Status: | Closed/Complete | Components: | Channels/chan_sip/General | ||
Versions: | 13.14.0 14.3.0 | Frequency of Occurrence | |||
Related Issues: |
| ||||
Environment: | Attachments: | ( 0) 0001-CDR-Protect-from-data-overflow-in-ast_cdr_setuserfie.patch | |||
Description: | While studying the channels/chan_sip.c from the Asterisk 13 branch I
found suspicious code that might enable a buffer overflow on reception of a SIP INFO packet. In channels/chan_sip.c the function handle_request_info() contains the following code: {noformat} } else if (!ast_strlen_zero(c = sip_get_header(req, "X-ClientCode"))) { /* Client code (from SNOM phone) */ if (ast_test_flag(&p->flags[0], SIP_USECLIENTCODE)) { if (p->owner) { ast_cdr_setuserfield(ast_channel_name(p->owner), c); } transmit_response(p, "200 OK", req); } else { transmit_response(p, "403 Forbidden", req); } return; {noformat} This code gets the content of the X-ClientCode header, and if the useclientcode has been enabled for the account, calls ast_cdr_setuserfield with the supplied value. In turn, ast_cdr_userfield contains the following: {noformat} if (cdr) { ao2_lock(cdr); for (it_cdr = cdr; it_cdr; it_cdr = it_cdr->next) { if (it_cdr->fn_table == &finalized_state_fn_table) { continue; } strcpy(it_cdr->party_a.userfield, userfield); } ao2_unlock(cdr); } {noformat} The strcpy() call has as a target a char array with a fixed size of 256 bytes. No length validation is apparent from the code. What exactly prevents a malicious remote client from sending a header that exceeds 256 bytes and overwrites the CDR userfield and whatever lies beyond it? | ||||
Comments: | By: Joshua C. Colp (jcolp) 2017-03-27 05:48:09.030-0500 This does require that the client code option be enabled, which is not the default. By: Corey Farrell (coreyfarrell) 2017-03-27 09:04:39.414-0500 Seems like the bug is in cdr.c? I mean {{char userfield\[AST_MAX_USER_FIELD];}} is a private member declared in cdr.c, doesn't seem like chan_sip's responsibility to protect cdr.c from itself. Also it appears func_cdr.c is effected as well? I realize dialplan functions are harder to get to but it's not registered as escalating so any AMI user could use it to crash Asterisk. By: Joshua C. Colp (jcolp) 2017-03-27 09:06:49.825-0500 Indeed. By: Friendly Automation (friendly-automation) 2017-04-04 06:23:34.573-0500 Change 5394 merged by Joshua Colp: CDR: Protect from data overflow in ast_cdr_setuserfield. [https://gerrit.asterisk.org/5394|https://gerrit.asterisk.org/5394] |