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 LinuxIndex: ssl_sock_gtls.c
===================================================================
--- ssl_sock_gtls.c (revision 5783)
+++ ssl_sock_gtls.c (working copy)
@@ -491,11 +491,16 @@
suite = gnutls_cipher_suite_info(i, (unsigned char *)id,
NULL, NULL, NULL, NULL);
- tls_ciphers[i].id = 0;
+
+ if (i < PJ_ARRAY_SIZE(tls_ciphers))
+ tls_ciphers[i].id = 0;
+ else
+ break;
+
/* usually the array size is bigger than the number of available
* ciphers anyway, so by checking here we can exit the loop as soon
* as either all ciphers have been added or the array is full */
- if (suite && i < PJ_ARRAY_SIZE(tls_ciphers)) {
+ if (suite) {
tls_ciphers[i].id = (pj_ssl_cipher)
(pj_uint32_t) ((id[0] << 8) | id[1]);
tls_ciphers[i].name = suite;
_______________________________________________
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@xxxxxxxxxxxxxxx
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
