1640041 - null pointer passed as argument 2, which is declared to never be null in security/nss/lib/ssl/sslencode.c:67

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Tyson Smith [:tsmith]]

To enable this check add the following to your mozconfig:

ac_add_options --enable-undefined-sanitizer="nonnull-attribute"

This can reproduced by running gtests:

[ RUN      ] TransportTest.TestConnectSrtp
src/security/nss/lib/ssl/sslencode.c:67:37: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:28: note: nonnull attribute specified here
    #0 0x7f6a9ef3d358 in sslBuffer_Append src/security/nss/lib/ssl/sslencode.c:67:5
    #1 0x7f6a9ef2d484 in ssl_CallCustomExtensionSenders src/security/nss/lib/ssl/ssl3ext.c:695:10
    #2 0x7f6a9ef2d484 in ssl_ConstructExtensions src/security/nss/lib/ssl/ssl3ext.c:803:14
    #3 0x7f6a9ef06065 in ssl3_SendClientHello src/security/nss/lib/ssl/ssl3con.c:5176:14
    #4 0x7f6a9ef3c2ae in ssl_BeginClientHandshake src/security/nss/lib/ssl/sslcon.c:189:10
    #5 0x7f6a9ef49564 in ssl_Do1stHandshake src/security/nss/lib/ssl/sslsecur.c:41:14
    #6 0x7f6a9ef49564 in SSL_ForceHandshake src/security/nss/lib/ssl/sslsecur.c:384:14
    #7 0x7f6a81b58d9e in mozilla::TransportLayerDtls::Handshake() src/media/mtransport/transportlayerdtls.cpp:888:18
    #8 0x7f6a81b5b7d9 in mozilla::TransportLayerDtls::TimerCallback(nsITimer*, void*) src/media/mtransport/transportlayerdtls.cpp:1471:9
    #9 0x7f6a7f9a93a7 in nsTimerImpl::Fire(int) src/xpcom/threads/nsTimerImpl.cpp:561:7
    #10 0x7f6a7f9a8d05 in nsTimerEvent::Run() src/xpcom/threads/TimerThread.cpp:250:11
    #11 0x7f6a7f9b8d95 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1211:14
    #12 0x7f6a7f9c117c in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:501:10
    #13 0x7f6a7fd025b1 in mozilla::net::nsSocketTransportService::Run() src/netwerk/base/nsSocketTransportService2.cpp:1134:11
    #14 0x7f6a7fd0425c in non-virtual thunk to mozilla::net::nsSocketTransportService::Run() src/netwerk/base/nsSocketTransportService2.cpp
    #15 0x7f6a7f9b8d95 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1211:14
    #16 0x7f6a7f9c117c in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:501:10
    #17 0x7f6a80c4dfd9 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:302:20
    #18 0x7f6a80b17917 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:315:10
    #19 0x7f6a80b17917 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:308:3
    #20 0x7f6a80b17917 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290:3
    #21 0x7f6a7f9b304e in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:444:10
    #22 0x7f6aa43707b7 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #23 0x7f6aa3fbe6da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #24 0x7f6aa2f9c88e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Comment 1

[J.C. Jones [:jcj] (he/him)]

Looks like tail.buf can remain null if there aren't any extensions asking to be last, so that memcpy or the call to sslBuffer_Append needs a guard. The null memcpy is safe though since the length to be copied ends up as zero, but as UBSan indicates, even though the bullet misses our foot, doesn't mean we should be aiming there.

Comment 2

[Martin Thomson [:mt:]]

Attachment: Bug 1640041 - Don't memcpy nothing, r?jcj

Depends on D76421

Comment 3

[Kevin Jacobs [:kjacobs]]

https://hg.mozilla.org/projects/nss/rev/8d7c96ab80a78c84214b46d4fc0b8ecfd5904cb3

Comment 4

[(Away)]

Just to connect the dots: this was merged from the nss repo to m-c in bug 1636656.