Closed Bug 1218326 (CVE-2015-7210) Opened 9 years ago Closed 9 years ago

UAF due to DataChannelConnection not Destroy()ed before deletion

Categories

(Core :: WebRTC, defect, P1)

44 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox41 --- wontfix
firefox42 --- wontfix
firefox43 + verified
firefox44 + fixed
firefox45 + fixed
firefox-esr38 43+ verified
b2g-v2.5 --- fixed
thunderbird_esr38 --- fixed

People

(Reporter: loobenyang, Assigned: bwc)

References

Details

(Keywords: csectype-uaf, regression, sec-critical, Whiteboard: [adv-main43+][adv-esr38.5+])

Attachments

(3 files, 2 obsolete files)

Attached file UAF_AutoSPSLock_Repro.html —
Reproduce test case:

<html><script>
var rtcConfig = { "iceServers": [{ "url": "stun:stun.l.google.com:19302" }] };
var pc0 = new RTCPeerConnection(rtcConfig);
var pc1 = new RTCPeerConnection(rtcConfig);
pc1.onsignalingstatechange = function (e) {dataChan0 = pc1.createDataChannel("DataChanName0");}; 
var dataChan0 = pc0.createDataChannel("DataChanName0");;
pc1.close();
setTimeout(function(){location.reload()},270);
</script></html>

Steps to reproduce:
1. Open the attached test case (UAF_AutoSPSLock_Repro.html) in Firefox browser.
2. Firefox crashes in js::AutoSPSLock::~AutoSPSLock():

First-chance exception at 0x0F7A536F (nss3.dll) in firefox.exe: 0xC0000005: Access violation reading location 0x5A5A5A62.
Unhandled exception at 0x0F7A536F (nss3.dll) in firefox.exe: 0xC0000005: Access violation reading location 0x5A5A5A62.


The address pattern 0x5A5A5A indicates an Use After Free.


Firefox version: 44.0a1 (2015-10-03)



Variables:

+		lock	0x5a5a5a5a {links={next=??? prev=??? } owner=??? waitQ={next=??? prev=??? } ...}	PRLock *
+		me	0x0769a000 {state=2 priority=PR_PRIORITY_NORMAL (1) arg=0x00000000 ...}	PRThread *
		me->flags	136	unsigned int
+		suspendAllThread	0x00000000 <NULL>	PRThread *


Registers:

EAX = 5A5A5A5A EBX = 00000001 ECX = FE4FD000 EDX = FFFFFFFF ESI = 011C39A0 EDI = 0019495C EIP = 0F7A536F ESP = 0115EC8C EBP = 0115EC90 EFL = 00010206 

0x5a5a5a62 = 00000000 


Call Stack:

>	nss3.dll!PR_Unlock(PRLock * lock) Line 322	C
 	xul.dll!js::AutoSPSLock::~AutoSPSLock() Line 221	C++
 	xul.dll!mozilla::BaseAutoLock<mozilla::Mutex>::~BaseAutoLock<mozilla::Mutex>() Line 170	C++
 	xul.dll!mozilla::DataChannelConnection::Close(mozilla::DataChannel * aChannel) Line 2495	C++
 	xul.dll!mozilla::DataChannel::Close() Line 2587	C++
 	xul.dll!nsDOMDataChannel::~nsDOMDataChannel() Line 50	C++
 	[External Code]	
 	xul.dll!mozilla::DOMEventTargetHelper::DeleteCycleCollectable() Line 81	C++
 	xul.dll!mozilla::DOMEventTargetHelper::cycleCollection::DeleteCycleCollectable(void * p) Line 64	C++
 	xul.dll!SnowWhiteKiller::~SnowWhiteKiller() Line 2657	C++
 	xul.dll!nsCycleCollector::FreeSnowWhite(bool aUntilNoSWInPurpleBuffer) Line 2823	C++
 	xul.dll!nsCycleCollector_doDeferredDeletion() Line 4081	C++
 	xul.dll!AsyncFreeSnowWhite::Run() Line 144	C++
 	xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 960	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 277	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate) Line 95	C++
 	xul.dll!MessageLoop::RunInternal() Line 235	C++
 	xul.dll!MessageLoop::RunHandler() Line 228	C++
 	xul.dll!MessageLoop::Run() Line 202	C++
 	xul.dll!nsBaseAppShell::Run() Line 158	C++
 	xul.dll!nsAppShell::Run() Line 178	C++
 	xul.dll!nsAppStartup::Run() Line 281	C++
 	xul.dll!XREMain::XRE_mainRun() Line 4298	C++
 	xul.dll!XREMain::XRE_main(int argc, char * * argv, const nsXREAppData * aAppData) Line 4391	C++
 	xul.dll!XRE_main(int argc, char * * argv, const nsXREAppData * aAppData, unsigned int aFlags) Line 4493	C++
 	firefox.exe!do_main(int argc, char * * argv, nsIFile * xreDirectory) Line 212	C++
 	firefox.exe!NS_internal_main(int argc, char * * argv) Line 399	C++
 	firefox.exe!wmain(int argc, wchar_t * * argv) Line 131	C++
 	[External Code]	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
This bug affects stable release. I ran the same test case with prefix webRTC (mozRTCPeerConnection) in Firefox 41.0.2, I got:

(31d4.e88): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=02e11140 ebx=00000066 ecx=e5e5e5e5 edx=00000003 esi=15803f10 edi=000004c8
eip=5f504ae9 esp=00e7f264 ebp=00e7f304 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010202
nss3!PR_Unlock+0x9:
5f504ae9 394108          cmp     dword ptr [ecx+8],eax ds:002b:e5e5e5ed=????????
0:000> !analyze -v

FAULTING_IP: 
nss3!PR_Unlock+9 [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\nsprpub\pr\src\threads\combined\prulock.c @ 322]
5f504ae9 394108          cmp     dword ptr [ecx+8],eax

EXCEPTION_RECORD:  ffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 5f504ae9 (nss3!PR_Unlock+0x00000009)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 00000000
   Parameter[1]: e5e5e5ed
Attempt to read from address e5e5e5ed

CONTEXT:  00000000 -- (.cxr 0x0;r)
eax=02e11140 ebx=00000066 ecx=e5e5e5e5 edx=00000003 esi=15803f10 edi=000004c8
eip=5f504ae9 esp=00e7f264 ebp=00e7f304 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010202
nss3!PR_Unlock+0x9:
5f504ae9 394108          cmp     dword ptr [ecx+8],eax ds:002b:e5e5e5ed=????????

FAULTING_THREAD:  00000e88

DEFAULT_BUCKET_ID:  INVALID_POINTER_READ

PROCESS_NAME:  firefox.exe

ERROR_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%08lx referenced memory at 0x%08lx. The memory could not be %s.

EXCEPTION_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%08lx referenced memory at 0x%08lx. The memory could not be %s.

EXCEPTION_PARAMETER1:  00000000

EXCEPTION_PARAMETER2:  e5e5e5ed

READ_ADDRESS:  e5e5e5ed 

FOLLOWUP_IP: 
nss3!PR_Unlock+9 [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\nsprpub\pr\src\threads\combined\prulock.c @ 322]
5f504ae9 394108          cmp     dword ptr [ecx+8],eax

NTGLOBALFLAG:  0

APPLICATION_VERIFIER_FLAGS:  0

APP:  firefox.exe

ANALYSIS_VERSION: 6.3.9600.17029 (debuggers(dbg).140219-1702) x86fre

PRIMARY_PROBLEM_CLASS:  INVALID_POINTER_READ

BUGCHECK_STR:  APPLICATION_FAULT_INVALID_POINTER_READ

LAST_CONTROL_TRANSFER:  from 5d657216 to 5f504ae9

STACK_TEXT:  
00e7f260 5d657216 e5e5e5e5 1329c1c0 5d6571f3 nss3!PR_Unlock+0x9
00e7f26c 5d6571f3 1329c1c0 0fdfd700 5dc6eaa3 xul!mozilla::DataChannelConnection::Close+0x21
00e7f278 5dc6eaa3 0fdfd700 5dc6f0fe 0f97e000 xul!mozilla::DataChannel::Close+0x35
00e7f280 5dc6f0fe 0f97e000 5d147244 00000001 xul!nsDOMDataChannel::~nsDOMDataChannel+0x55
00e7f288 5d147244 00000001 5d181649 0fdfd700 xul!nsDOMDataChannel::`scalar deleting destructor'+0x8
00e7f290 5d181649 0fdfd700 5d12566d 5f25edec xul!mozilla::DOMEventTargetHelper::DeleteCycleCollectable+0xf
00e7f298 5d12566d 5f25edec 0fdfd700 02e890a0 xul!nsGlobalWindow::cycleCollection::DeleteCycleCollectable+0xa
00e7f2c8 5d269284 02e47cf4 08eff7f0 00000000 xul!SnowWhiteKiller::~SnowWhiteKiller+0x87
00e7f310 5d37a6bb 00000000 5d37a63f 08eff7f0 xul!nsCycleCollector::FreeSnowWhite+0x1c4
00e7f318 5d37a63f 08eff7f0 00000000 773e4530 xul!nsCycleCollector_doDeferredDeletion+0x15
00e7f364 5d2be245 08eff7f0 02e444a0 02e44490 xul!AsyncFreeSnowWhite::Run+0x1c
00e7f474 5d2bd14f 02e460f0 00000000 00e7f4ac xul!nsThread::ProcessNextEvent+0x281
00e7f4a4 5d2bc66a 02e7b001 8251d076 02e47cf0 xul!mozilla::ipc::MessagePump::Run+0x5f
00e7f4dc 5d2bbf10 02e460f0 00000001 5d14ca00 xul!MessageLoop::RunHandler+0x20
00e7f4fc 5d2bd792 09368d00 00000000 5d2bdbc4 xul!MessageLoop::Run+0x19
00e7f508 5d2bdbc4 02e47cf0 09368d00 5d2a12f4 xul!nsBaseAppShell::Run+0x32
00e7f514 5d2a12f4 02e47cf0 00e7f871 0cb75b40 xul!nsAppShell::Run+0x1b
00e7f524 5d37e63b 09368d00 00e7f778 02e1d3c0 xul!nsAppStartup::Run+0x20
00e7f708 5d37e8aa 00e7f7c0 00000001 00e7f8c0 xul!XREMain::XRE_mainRun+0x4a7
00e7f724 5d608326 00000000 00000000 00000000 xul!XREMain::XRE_main+0x1f8
00e7f878 00971635 00000001 00ef4718 00e7f8c0 xul!XRE_main+0x40
00e7fa0c 009712dc 02e1c1c0 00ef3db8 00000960 firefox!do_main+0x125
00e7faa0 009710dc 00972521 00972521 00e7fafc firefox!NS_internal_main+0xec
00e7fab4 009724a4 00ef4718 00f06368 00f063e0 firefox!wmain+0xbc
00e7fafc 74d47c04 fed2f000 74d47be0 aafe3d21 firefox!__tmainCRTStartup+0xfe
00e7fb10 773fad1f fed2f000 a954b858 00000000 KERNEL32!BaseThreadInitThunk+0x24
00e7fb58 773facea ffffffff 773e0227 00000000 ntdll!__RtlUserThreadStart+0x2f
00e7fb68 00000000 00972521 fed2f000 00000000 ntdll!_RtlUserThreadStart+0x1b


STACK_COMMAND:  .cxr 0x0 ; kb

FAULTING_SOURCE_LINE:  c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\nsprpub\pr\src\threads\combined\prulock.c

FAULTING_SOURCE_FILE:  c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\nsprpub\pr\src\threads\combined\prulock.c

FAULTING_SOURCE_LINE_NUMBER:  322

SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  nss3!PR_Unlock+9

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: nss3

IMAGE_NAME:  nss3.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  561ee570

FAILURE_BUCKET_ID:  INVALID_POINTER_READ_c0000005_nss3.dll!PR_Unlock

BUCKET_ID:  APPLICATION_FAULT_INVALID_POINTER_READ_nss3!PR_Unlock+9

ANALYSIS_SOURCE:  UM

FAILURE_ID_HASH_STRING:  um:invalid_pointer_read_c0000005_nss3.dll!pr_unlock

FAILURE_ID_HASH:  {8f593d00-4106-1fc2-2dfa-dd6ab7d5f56d}

Followup: MachineOwner
---------
FYI. I ran the same test case UAF_AutoSPSLock_Repro.html in official Linux ASAN build, it did report a Use After Free in ~BaseAutoLock:

Firefox version: 44.0a1 (2015-10-12)

=================================================================
==2349==ERROR: AddressSanitizer: heap-use-after-free on address 0x6150007ff158 at pc 0x7f7ec917b0d9 bp 0x7fff9f093f80 sp 0x7fff9f093f78
READ of size 8 at 0x6150007ff158 thread T0 (Web Content)
    #0 0x7f7ec917b0d8 in Unlock /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/netwerk/sctp/datachannel/../../../dist/include/mozilla/Mutex.h:75
    #1 0x7f7ec917b0d8 in ~BaseAutoLock /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/netwerk/sctp/datachannel/../../../dist/include/mozilla/Mutex.h:169
    #2 0x7f7ec917b0d8 in Close /builds/slave/m-cen-l64-asan-000000000000000/build/src/netwerk/sctp/datachannel/DataChannel.cpp:2495
    #3 0x7f7ec917b0d8 in mozilla::DataChannel::Close() /builds/slave/m-cen-l64-asan-000000000000000/build/src/netwerk/sctp/datachannel/DataChannel.cpp:2586
    #4 0x7f7ecad99a2c in nsDOMDataChannel::~nsDOMDataChannel() /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsDOMDataChannel.cpp:49
    #5 0x7f7ecad99afd in nsDOMDataChannel::~nsDOMDataChannel() /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsDOMDataChannel.cpp:43
    #6 0x7f7ec8808e08 in SnowWhiteKiller::~SnowWhiteKiller() /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2655
    #7 0x7f7ec8808a2e in nsCycleCollector::FreeSnowWhite(bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2823
    #8 0x7f7ec9d32019 in AsyncFreeSnowWhite::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/src/XPCJSRuntime.cpp:144
    #9 0x7f7ec891b7ef in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:972
    #10 0x7f7ec8994c6a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:297
    #11 0x7f7ec923f509 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:95
    #12 0x7f7ec91adaec in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #13 0x7f7ec91adaec in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:227
    #14 0x7f7ec91adaec in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #15 0x7f7ece2ef557 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156
    #16 0x7f7ed0148d42 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:785
    #17 0x7f7ec91adaec in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #18 0x7f7ec91adaec in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:227
    #19 0x7f7ec91adaec in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #20 0x7f7ed014842c in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:621
    #21 0x48d740 in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:237
    #22 0x7f7ec6289ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #23 0x48ca9c in _start (/home/parnell/FirefoxBuilds/firefox/plugin-container+0x48ca9c)

0x6150007ff158 is located 88 bytes inside of 456-byte region [0x6150007ff100,0x6150007ff2c8)
freed by thread T0 (Web Content) here:
    #0 0x474eb1 in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    #1 0x7f7ec91670ac in mozilla::DataChannelConnection::Release() /builds/slave/m-cen-l64-asan-000000000000000/build/src/netwerk/sctp/datachannel/DataChannel.cpp:295
    #2 0x7f7ec917057f in mozilla::DataChannelConnection::CloseInt(mozilla::DataChannel*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/netwerk/sctp/datachannel/DataChannel.cpp:2532
    #3 0x7f7ec917b02b in Close /builds/slave/m-cen-l64-asan-000000000000000/build/src/netwerk/sctp/datachannel/DataChannel.cpp:2494
    #4 0x7f7ec917b02b in mozilla::DataChannel::Close() /builds/slave/m-cen-l64-asan-000000000000000/build/src/netwerk/sctp/datachannel/DataChannel.cpp:2586
    #5 0x7f7ecad99a2c in nsDOMDataChannel::~nsDOMDataChannel() /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsDOMDataChannel.cpp:49
    #6 0x7f7ecad99afd in nsDOMDataChannel::~nsDOMDataChannel() /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsDOMDataChannel.cpp:43
    #7 0x7f7ec8808e08 in SnowWhiteKiller::~SnowWhiteKiller() /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2655
    #8 0x7f7ec8808a2e in nsCycleCollector::FreeSnowWhite(bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2823
    #9 0x7f7ec9d32019 in AsyncFreeSnowWhite::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/src/XPCJSRuntime.cpp:144
    #10 0x7f7ec891b7ef in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:972
    #11 0x7f7ec8994c6a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:297
    #12 0x7f7ec923f509 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:95
    #13 0x7f7ec91adaec in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #14 0x7f7ec91adaec in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:227
    #15 0x7f7ec91adaec in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #16 0x7f7ece2ef557 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156
    #17 0x7f7ed0148d42 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:785
    #18 0x7f7ec91adaec in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #19 0x7f7ec91adaec in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:227
    #20 0x7f7ec91adaec in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #21 0x7f7ed014842c in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:621
    #22 0x48d740 in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:237
    #23 0x7f7ec6289ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

previously allocated by thread T0 (Web Content) here:
    #0 0x4750b1 in __interceptor_malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x48dd5d in moz_xmalloc /builds/slave/m-cen-l64-asan-000000000000000/build/src/memory/mozalloc/mozalloc.cpp:83
    #2 0x7f7ec9fff83f in operator new /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/media/webrtc/signaling/signaling_ecc/../../../../dist/include/mozilla/mozalloc.h:186
    #3 0x7f7ec9fff83f in mozilla::PeerConnectionImpl::EnsureDataConnection(unsigned short) /builds/slave/m-cen-l64-asan-000000000000000/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:1114
    #4 0x7f7eca001905 in mozilla::PeerConnectionImpl::CreateDataChannel(nsAString_internal const&, nsAString_internal const&, unsigned short, bool, unsigned short, unsigned short, bool, unsigned short, nsDOMDataChannel**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:1342
    #5 0x7f7eca001726 in mozilla::PeerConnectionImpl::CreateDataChannel(nsAString_internal const&, nsAString_internal const&, unsigned short, bool, unsigned short, unsigned short, bool, unsigned short, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:1314
    #6 0x7f7ecb6ae6db in mozilla::dom::PeerConnectionImplBinding::createDataChannel(JSContext*, JS::Handle<JSObject*>, mozilla::PeerConnectionImpl*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/./PeerConnectionImplBinding.cpp:1162
    #7 0x7f7ecca0d437 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/bindings/BindingUtils.cpp:2691
    #8 0x7f7ed200ed0b in CallJSNative /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jscntxtinlines.h:235
    #9 0x7f7ed200ed0b in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:772
    #10 0x7f7ed2059cbd in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:839
    #11 0x7f7ed25c5bff in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jit/BaselineIC.cpp:9033
    #12 0x7f7ed28709a6 in EnterBaseline(JSContext*, js::jit::EnterJitData&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jit/BaselineJIT.cpp:126
    #13 0x7f7ed28702ed in js::jit::EnterBaselineMethod(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jit/BaselineJIT.cpp:161
    #14 0x7f7ed2026892 in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:715
    #15 0x7f7ed200f46f in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:802
    #16 0x7f7ed2059cbd in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:839
    #17 0x7f7ed1ca2a45 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jsapi.cpp:4769
    #18 0x7f7ecb86b5fa in mozilla::dom::RTCPeerConnectionJSImpl::CreateDataChannel(nsAString_internal const&, mozilla::dom::RTCDataChannelInit const&, mozilla::ErrorResult&, JSCompartment*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/./RTCPeerConnectionBinding.cpp:6671
    #19 0x7f7ecb89f832 in operator _Bool /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/./RTCPeerConnectionBinding.cpp:8055
    #20 0x7f7ecb89f832 in mozilla::dom::RTCPeerConnectionBinding::createDataChannel(JSContext*, JS::Handle<JSObject*>, mozilla::dom::RTCPeerConnection*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/./RTCPeerConnectionBinding.cpp:4275
    #21 0x7f7ecca0d437 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/bindings/BindingUtils.cpp:2691
    #22 0x7f7ed200ed0b in CallJSNative /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jscntxtinlines.h:235
    #23 0x7f7ed200ed0b in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:772
    #24 0x7f7ed204732d in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:3107
    #25 0x7f7ed2026a70 in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:725
    #26 0x7f7ed200f46f in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:802
    #27 0x7f7ed2059cbd in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:839
    #28 0x7f7ed1ee5925 in js::DirectProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/proxy/DirectProxyHandler.cpp:77
    #29 0x7f7ed1ed34f9 in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:289
    #30 0x7f7ec9c3f2e5 in xpc::JSXrayTraits::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&, js::Wrapper const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/wrappers/XrayWrapper.h:244

SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/netwerk/sctp/datachannel/../../../dist/include/mozilla/Mutex.h:75 Unlock
Shadow bytes around the buggy address:
  0x0c2a800f7dd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2a800f7de0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2a800f7df0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2a800f7e00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2a800f7e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c2a800f7e20: fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd
  0x0c2a800f7e30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2a800f7e40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2a800f7e50: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
  0x0c2a800f7e60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2a800f7e70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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 redzon==2349==ABORTING

###!!! [Parent][MessageChannel] Error: (msgtype=0x280082,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv
Can you look at this, Randell? Thanks.
Group: core-security → media-core-security
Flags: needinfo?(rjesup)
Flags: sec-bounty?
Assignee: nobody → rjesup
backlog: --- → webrtc/webaudio+
Rank: 10
Flags: needinfo?(rjesup)
Priority: -- → P1
[Tracking Requested - why for this release]: sec-crit.  Likely too late for 42, though the fix is likely simple, so not marking wontfix for 42 yet.

The fundamental problem is that in bug 970690, CloseInt() was no longer called directly from ~PeerConnectionImpl(), instead it calls Close(), which indirectly calls CloseInt() if the status is Closed.  Note that CloseInt() is what calls DataChannelConnection::Destroy(), which we require to be called before destruction.

However, the problem is that there are paths where the status (at ~PeerConnectionImpl() time) is already Closed, bu the DataChannelConnection is still alive (i.e. Destroy() hasn't been called for it).

This causes CloseInt() to not get called, which means Destroy doesn't get called, and when we delete the DataChannelConnection on the last reference's release, we either UAF or (in debug builds) MOZ_ASSERT().

This can be wallpapered fairly easily in several ways.  The simplest would be to clone the code to Destroy() and null out mDataConnection from CloseInt() to ~PeerConnectionImpl.  This guarantees it's shut down before we call Close().  As a followup, we should figure out why we can get to this call with a mSignalingState of SignalingClosed, but without having called CloseInt.

Note: also affects B2G (most if not all versions) as this was landed in Fx31.

NI byron in general, NI Al about 42.
Rank: 10 → 5
Flags: needinfo?(docfaraday)
Flags: needinfo?(abillings)
OS: Windows 8.1 → All
Hardware: x86 → All
Summary: Use After Free in ~AutoSPSLock() → UAF due to DataChannelConnection not Destroy()ed before deletion
Attached patch Ensure DataConnection is Destroyed (obsolete) — — Splinter Review
wallpaper patch, with assertion - may not be good to land with the assertion enabled - may use for safe uplifts, without the assert, and fix the root cause in nightly
Attachment #8679843 - Flags: review?(docfaraday)
4 hours of running the testcase with the patch with no problems.
(In reply to Randell Jesup [:jesup] from comment #4)
> [Tracking Requested - why for this release]: sec-crit.  Likely too late for
> 42, though the fix is likely simple, so not marking wontfix for 42 yet.
> 
> The fundamental problem is that in bug 970690, CloseInt() was no longer
> called directly from ~PeerConnectionImpl(), instead it calls Close(), which
> indirectly calls CloseInt() if the status is Closed.  Note that CloseInt()
> is what calls DataChannelConnection::Destroy(), which we require to be
> called before destruction.
> 

   The only place that updates PeerConnectionImpl::mSignalingState is PeerConnectionImpl::SetSignalingState_m, which is also the only place that calls PeerConnectionImpl::CloseInt (and always does so if the state changed to Closed). I don't see how we could possibly end up in state Closed without a call to CloseInt unless ~PeerConnectionImpl were invoked somewhere between the place where SetSiganlingState_m sets the signaling state to closed, and the place where CloseInt() calls mDataConnection::Destroy.

   I think we're actually messing up here: https://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp#1291

   This code was written with the understanding that if EnsureDataConnection failed, there were no resources that needed to be cleaned up with Destroy, but that may not be the case.
Flags: needinfo?(docfaraday)
I need to get a build on my linux box so I can get up-to-date line numbers for the ASAN stacks (the line numbers around the datachannel stuff in comment 2 do not appear to be up-to-date).
Yeah, this is too late for 42 at this point. This would need sec-approval to land, which would be allowed around November 10 to be into the next cycle.
Flags: needinfo?(abillings)
Ok, the actual problem is we are ending up here when the PC is already closed:

https://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp?case=true&from=PeerConnectionImpl.cpp#1111

This is obviously busted.
Is there any reason that we should allow any of these datachannel functions to work if the PC is closed? My inclination is to convert the PC_AUTO_ENTER_API_CALL_NO_CHECK() calls to PC_AUTO_ENTER_API_CALL()
[Tracking Requested - why for this release]: sec-critical
Assignee: rjesup → docfaraday
Status: NEW → ASSIGNED
Comment on attachment 8680125 [details] [diff] [review]
Prevent datachannel operations on closed PeerConnections

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b26db8b66ac6
Attachment #8680125 - Attachment filename: . → datachannel_check_state.patch
Attachment #8680125 - Flags: review?(rjesup)
Comment on attachment 8680125 [details] [diff] [review]
Prevent datachannel operations on closed PeerConnections

Review of attachment 8680125 [details] [diff] [review]:
-----------------------------------------------------------------

I'd love to resolve the khuey thing; I made a few tries at it and all failed.  You could ask him his suggestion on how to tweak the APIs to simplify this.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ +1430,5 @@
>  
>  void
>  PeerConnectionImpl::NotifyDataChannel(already_AddRefed<DataChannel> aChannel)
>  {
> +  PC_AUTO_ENTER_API_CALL_VOID_RETURN(false);

This will leak the channel (and thus the entire DataChannelConnection)... (on failure).
Dangers of hidden returns...  Note that releasing this on failure will be a bit painful due to khuey's note; I fiddled with alternatives a bit with no luck.
Attachment #8680125 - Flags: review?(rjesup) → review-
(In reply to Randell Jesup [:jesup] from comment #16)
> Comment on attachment 8680125 [details] [diff] [review]
> Prevent datachannel operations on closed PeerConnections
> 
> Review of attachment 8680125 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'd love to resolve the khuey thing; I made a few tries at it and all
> failed.  You could ask him his suggestion on how to tweak the APIs to
> simplify this.
> 
> ::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
> @@ +1430,5 @@
> >  
> >  void
> >  PeerConnectionImpl::NotifyDataChannel(already_AddRefed<DataChannel> aChannel)
> >  {
> > +  PC_AUTO_ENTER_API_CALL_VOID_RETURN(false);
> 
> This will leak the channel (and thus the entire DataChannelConnection)...
> (on failure).
> Dangers of hidden returns...  Note that releasing this on failure will be a
> bit painful due to khuey's note; I fiddled with alternatives a bit with no
> luck.

Looking at the function again, I guess it won't be the end of the world if it runs in Closed.
Attachment #8680125 - Attachment is obsolete: true
Comment on attachment 8680596 [details] [diff] [review]
Prevent datachannel operations on closed PeerConnections

https://treeherder.mozilla.org/#/jobs?repo=try&revision=215f7a752ab5
Attachment #8680596 - Attachment filename: . → datachannel_check_state.patch
Attachment #8680596 - Flags: review?(rjesup)
Attachment #8680596 - Flags: review?(rjesup) → review+
Check back on try sometime in the next month :/
Flags: needinfo?(docfaraday)
Attachment #8679843 - Flags: review?(docfaraday)
Assuming this affects 45 as well. Tracking for 43+ since this is sec-critical.
Not for 42.  Will land later in the cycle.
Comment on attachment 8680596 [details] [diff] [review]
Prevent datachannel operations on closed PeerConnections

Approval Request Comment
[Feature/regressing bug #]:

   Bug 798825

[User impact if declined]:

   Content code will be able to cause a UAF pretty reliably.

[Describe test coverage new/current, TreeHerder]:

   We have the usual datachannel tests, but nothing that targets this specific bug.

[Risks and why]: 

   Almost risk-free; all we're doing is adding some additional sanity-checking.

[String/UUID change made/needed]:

   None.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

   It would be pretty easy to write code that triggers the UAF. Taking advantage of it is perhaps harder.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

   Not really.

Which older supported branches are affected by this flaw?

   All.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

   I expect that this patch will apply cleanly, but if it doesn't, it will be very easy to backport.

How likely is this patch to cause regressions; how much testing does it need?
Flags: needinfo?(docfaraday)
Attachment #8680596 - Flags: sec-approval?
Attachment #8680596 - Flags: approval-mozilla-beta?
Attachment #8680596 - Flags: approval-mozilla-aurora?
Sec-approval+ for checkin on Nov 17, two weeks into the cycle. This is to reduce the time window of vulnerability after the checkin.
Whiteboard: [checkin on 11/17]
Attachment #8680596 - Flags: sec-approval? → sec-approval+
Let's check this in now for it to go to build. Then it will release on Nov. 17.
Moving date.
Whiteboard: [checkin on 11/17] → [checkin on 11/16]
Comment on attachment 8680596 [details] [diff] [review]
Prevent datachannel operations on closed PeerConnections

Fix for memory issue, ok to land on aurora and beta today.
Attachment #8680596 - Flags: approval-mozilla-beta?
Attachment #8680596 - Flags: approval-mozilla-beta+
Attachment #8680596 - Flags: approval-mozilla-aurora?
Attachment #8680596 - Flags: approval-mozilla-aurora+
Comment on attachment 8679843 [details] [diff] [review]
Ensure DataConnection is Destroyed

Marking obsolete based on talking with :jesup
Attachment #8679843 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/a7637b62d9b5
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Group: media-core-security → core-security-release
Flags: sec-bounty? → sec-bounty+
Whiteboard: [checkin on 11/16]
(In reply to Byron Campen [:bwc] from comment #24)
> Comment on attachment 8680596 [details] [diff] [review]
> Prevent datachannel operations on closed PeerConnections
> 
> Approval Request Comment
> [Feature/regressing bug #]:
> 
>    Bug 798825
> 
> [User impact if declined]:
> 
>    Content code will be able to cause a UAF pretty reliably.
> 
> [Describe test coverage new/current, TreeHerder]:
> 
>    We have the usual datachannel tests, but nothing that targets this
> specific bug.
> 
> [Risks and why]: 
> 
>    Almost risk-free; all we're doing is adding some additional
> sanity-checking.
> 
> [String/UUID change made/needed]:
> 
>    None.
> 
> [Security approval request comment]
> How easily could an exploit be constructed based on the patch?
> 
>    It would be pretty easy to write code that triggers the UAF. Taking
> advantage of it is perhaps harder.
> 
> Do comments in the patch, the check-in comment, or tests included in the
> patch paint a bulls-eye on the security problem?
> 
>    Not really.
> 
> Which older supported branches are affected by this flaw?
> 
>    All.
> 
> Do you have backports for the affected branches? If not, how different, hard
> to create, and risky will they be?
> 
>    I expect that this patch will apply cleanly, but if it doesn't, it will
> be very easy to backport.
> 
> How likely is this patch to cause regressions; how much testing does it need?

Thanks for the quick fix.

When you are saying "Taking advantage of it is perhaps harder.". 
Did you mean this UAF bug is much harder to exploit then other UAF bugs? or
just writing a exploit is harder than writing a test case to trigger the crash?
Flags: needinfo?(docfaraday)
(In reply to Looben Yang from comment #34)
> Thanks for the quick fix.
> 
> When you are saying "Taking advantage of it is perhaps harder.". 
> Did you mean this UAF bug is much harder to exploit then other UAF bugs? or
> just writing a exploit is harder than writing a test case to trigger the
> crash?

   The second one.
Flags: needinfo?(docfaraday)
Flags: sec-bounty+ → sec-bounty?
Attachment #8679042 - Attachment mime type: text/plain → text/html
We need this checked in on ESR, too (according to comment 24). Does it need a different patch? If not please request ESR approval on this patch.
Blocks: 798825
Flags: needinfo?(docfaraday)
Keywords: regression
Comment on attachment 8680596 [details] [diff] [review]
Prevent datachannel operations on closed PeerConnections

See other approval comments. Patch should apply cleanly.
Flags: needinfo?(docfaraday)
Attachment #8680596 - Flags: approval-mozilla-esr38?
FWIW in 42.0 I get a less interesting null-deref crash using the "41.0.2" version of the testcase
bp-0853d222-7717-4e56-8b3f-41cec2151125
bp-6930f920-1a44-4470-89d0-5d0792151125
(Mac vs Windows, though)

The 0x5a5a5a5a fill value was explicitly chosen (and processor cycles "wasted" writing it) explicitly to make people crash rather than have Firefox carry on and use the freed block. A bad thing has happened, but in order to exploit it you'd have to find a way to re-allocate that memory before we trip up over the poison value. Given the apparently racy nature of this UAF (took multiple, varying number of reload loops before it crashed) it's not clear from this testcase that re-allocation could be triggered in the vulnerability window.
Comment on attachment 8680596 [details] [diff] [review]
Prevent datachannel operations on closed PeerConnections

Sec critical, taking it.
Attachment #8680596 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Flags: sec-bounty? → sec-bounty+
leaving bounty at $4500 for now due to this use after free hitting poisoned memory. if extra research could make this more reliable or work around the mitigations in place we could reconsider a higher bounty amount.
Whiteboard: [adv-main43+][adv-esr38.5+]
Reproduced with Nightly from 2015-10-27 - after loading the attached testcase, the tab(s) crash:
> bp-6921bd04-d3b9-4e52-9dfc-d577b2151208
> bp-6407bbe8-45eb-491c-9c2c-d95032151208

No crash encountered with 43.0b9 (Build ID: 20151203163240) and latest ESR 38 tinderbox build (Build ID: 20151205001501), across platforms [1].

[1] Ubuntu 14.04 32-bit, Windows 7 64-bit and Mac OS X 10.11
Alias: CVE-2015-7210
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.