[pjsip] [PATCH] global-buffer-overflow in tls_init (pjlib/src/pj/ssl_sock_gtls.c)

Hugo Lefeuvre hugo.lefeuvre at savoirfairelinux.com
Thu Apr 19 14:43:58 EDT 2018


Dear pjsip developers,

The latest trunk revision 5778 is vulnerable to a global buffer overflow (tls_init function in pjlib/src/pj/ssl_sock_gtls.c).

This bug may be triggered by remote attackers to crash the client.

ASan stacktrace:

=================================================================
==23860==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7ffff6a70b60 at pc 0x7ffff51fc301 bp 0x7fffffffc860 sp 0x7fffffffc850
WRITE of size 4 at 0x7ffff6a70b60 thread T0
    #0 0x7ffff51fc300 in tls_init ../src/pj/ssl_sock_gtls.c:480
    #1 0x7ffff5203212 in tls_ciphers_fill ../src/pj/ssl_sock_gtls.c:2006
    #2 0x7ffff52036a6 in pj_ssl_cipher_name ../src/pj/ssl_sock_gtls.c:2113
    #3 0x7ffff4d356ec in ring::SIPCall::getDetails[abi:cxx11]() const /home/hlefeuvre/Development/ring-daemon/src/sip/sipcall.cpp:1134
    #4 0x7ffff4c08d7f in ring::Manager::getCallDetails(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/hlefeuvre/Development/ring-daemon/src/manager.cpp:2851
    #5 0x7ffff4e32489 in DRing::getCallDetails(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/hlefeuvre/Development/ring-daemon/src/client/callmanager.cpp:289
    #6 0x4ff429 in DBusCallManager::getCallDetails(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/hlefeuvre/Development/ring-daemon/bin/dbus/dbuscallmanager.cpp:93
    #7 0x508af4 in cx::ring::Ring::CallManager_adaptor::_getCallDetails_stub(DBus::CallMessage const&) /home/hlefeuvre/Development/ring-daemon/bin/dbus/dbuscallmanager.adaptor.h:1045
    #8 0x50b03b in DBus::Callback<cx::ring::Ring::CallManager_adaptor, DBus::Message, DBus::CallMessage const&>::call(DBus::CallMessage const&) const /home/hlefeuvre/Development/ring-daemon/contrib/x86_64-linux-gnu/include/dbus-c++-1/dbus-c++/util.h:283
    #9 0x53c696 in DBus::Slot<DBus::Message, DBus::CallMessage const&>::call(DBus::CallMessage const&) const (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x53c696)
    #10 0x53b789 in DBus::InterfaceAdaptor::dispatch_method(DBus::CallMessage const&) (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x53b789)
    #11 0x54410e in DBus::ObjectAdaptor::handle_message(DBus::Message const&) (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x54410e)
    #12 0x543685 in DBus::ObjectAdaptor::Private::message_function_stub(DBusConnection*, DBusMessage*, void*) (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x543685)
    #13 0x7ffff6c3f812  (/lib/x86_64-linux-gnu/libdbus-1.so.3+0x21812)
    #14 0x7ffff6c30d93 in dbus_connection_dispatch (/lib/x86_64-linux-gnu/libdbus-1.so.3+0x12d93)
    #15 0x5331a9 in DBus::Connection::Private::do_dispatch() (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x5331a9)
    #16 0x536110 in DBus::Dispatcher::dispatch_pending(std::__cxx11::list<DBus::Connection::Private*, std::allocator<DBus::Connection::Private*> >&) (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x536110)
    #17 0x535f0e in DBus::Dispatcher::dispatch_pending() (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x535f0e)
    #18 0x539c61 in DBus::BusDispatcher::do_iteration() (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x539c61)
    #19 0x53990f in DBus::BusDispatcher::enter() (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x53990f)
    #20 0x4930fc in DBusClient::event_loop() /home/hlefeuvre/Development/ring-daemon/bin/dbus/dbusclient.cpp:250
    #21 0x48d0d0 in main /home/hlefeuvre/Development/ring-daemon/bin/main.cpp:236
    #22 0x7ffff35ed82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #23 0x48c718 in _start (/home/hlefeuvre/Development/ring-daemon/bin/.libs/lt-dring+0x48c718)

0x7ffff6a70b60 is located 0 bytes to the right of global variable 'tls_ciphers' defined in '../src/pj/ssl_sock_gtls.c:207:3' (0x7ffff6a70520) of size 1600
0x7ffff6a70b60 is located 32 bytes to the left of global variable 'tls_last_error' defined in '../src/pj/ssl_sock_gtls.c:210:12' (0x7ffff6a70b80) of size 4
SUMMARY: AddressSanitizer: global-buffer-overflow ../src/pj/ssl_sock_gtls.c:480 tls_init
Shadow bytes around the buggy address:
  0x10007ed46110: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007ed46120: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007ed46130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007ed46140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007ed46150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10007ed46160: 00 00 00 00 00 00 00 00 00 00 00 00[f9]f9 f9 f9
  0x10007ed46170: 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x10007ed46180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007ed46190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007ed461a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007ed461b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
==23860==ABORTING

Please, note that this stacktrace is coming from a slightly older version of pjsip, so line numbers are not exactly corresponding to the ones from the latest svn revision, but the bug is still present.

Analysis:

At line 494, tls_init writes a 0 to the id field of tls_ciphers[i] before checking for i < PJ_ARRAY_SIZE(tls_ciphers):

466	/* Initialize GnuTLS. */
467	static pj_status_t tls_init(void)
468	{
[snip]
488	        for (i = 0; ; i++) {
489	            unsigned char id[2];
490	            const char *suite;[snip]
491	           
492	            suite = gnutls_cipher_suite_info(i, (unsigned char *)id,
493	                                             NULL, NULL, NULL, NULL);
494	            tls_ciphers[i].id = 0;
495	            /* usually the array size is bigger than the number of available
496	             * ciphers anyway, so by checking here we can exit the loop as soon
497	             * as either all ciphers have been added or the array is full */
498	            if (suite && i < PJ_ARRAY_SIZE(tls_ciphers)) {
[snip]
504	        }
505	
506	        tls_available_ciphers = i;
507	    }
508	
509	    return PJ_SUCCESS;
510	}

In this case, PJ_ARRAY_SIZE(tls_ciphers) is 100 and i = 100, so tls_ciphers[100].id = 0 is OOB write.

Patch:

In the attached patch we add a check making sure that i is smaller than PJ_ARRAY_SIZE(tls_ciphers) before updating tls_ciphers[i]. This should be enough to fix the issue.

Thanks for your work.

Best regards,
 Hugo Lefeuvre
 Ring team @Savoir-faire Linux
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gbo-tls_init.patch
Type: text/x-patch
Size: 991 bytes
Desc: not available
URL: <http://lists.pjsip.org/pipermail/pjsip_lists.pjsip.org/attachments/20180419/87a06474/attachment.patch>


More information about the pjsip mailing list