Asterisk
  1. Asterisk
  2. ASTERISK-26897

chan_sip: Security vulnerability with client code header

    Details

      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:

           } 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;
      

      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:

           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);
           }
      

      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?

        Issue Links

          Activity

          Hide
          Joshua Colp added a comment -

          This does require that the client code option be enabled, which is not the default.

          Show
          Joshua Colp added a comment - This does require that the client code option be enabled, which is not the default.
          Hide
          Corey Farrell added a comment -

          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.

          Show
          Corey Farrell added a comment - 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.
          Hide
          Joshua Colp added a comment -

          Indeed.

          Show
          Joshua Colp added a comment - Indeed.
          Hide
          Friendly Automation added a comment -

          Change 5394 merged by Joshua Colp:
          CDR: Protect from data overflow in ast_cdr_setuserfield.

          https://gerrit.asterisk.org/5394

          Show
          Friendly Automation added a comment - Change 5394 merged by Joshua Colp: CDR: Protect from data overflow in ast_cdr_setuserfield. https://gerrit.asterisk.org/5394

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development