[Home]

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:30Date Closed:2017-04-04 06:23:32
Priority:MajorRegression?
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:13.14.0 14.3.0 Frequency of
Occurrence
Related
Issues:
is related toASTERISK-27337 chan_sip: Security vulnerability with client code header (revisited)
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]