Bug 1423086 (CVE-2018-5091)

Use After Free in PeerConnectionImpl::DTMFSendTimerCallback_m()

VERIFIED FIXED

Status

()

P1
critical
Rank:
3
VERIFIED FIXED
a year ago
7 months ago

People

(Reporter: loobenyang, Assigned: bwc)

Tracking

({csectype-uaf, sec-critical})

59 Branch
csectype-uaf, sec-critical
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox-esr5258+ verified, firefox57 wontfix, firefox58+ verified, firefox59+ verified)

Details

(Whiteboard: [fixed by bug 1425901][adv-main58+][adv-esr52.6+][post-critsmash-triage])

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

a year ago
Reproduction test case: UAF_DTMFSendTimerCallback_m_PoC.html

Steps to reproduce: 
	1. Open UAF_DTMFSendTimerCallback_m_PoC.html in Firefox browser.
    2. Firefox crashes in PeerConnectionImpl::DTMFSendTimerCallback_m() by accessing freed memory.

		=================================================================
		==9169==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000198b88 at pc 0x7f94ab7d4e10 bp 0x7ffc550cc950 sp 0x7ffc550cc948
		READ of size 4 at 0x606000198b88 thread T0 (file:// Content)
			#0 0x7f94ab7d4e0f in IsEmpty /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTStringRepr.h:186:12
			#1 0x7f94ab7d4e0f in mozilla::PeerConnectionImpl::DTMFSendTimerCallback_m(nsITimer*, void*) /builds/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:3793

Firefox version: 59.0a1 (2017-12-04) (64-bit)
OS: Ubuntu 14.04 LTS

Stack trace:


	=================================================================
	==9169==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000198b88 at pc 0x7f94ab7d4e10 bp 0x7ffc550cc950 sp 0x7ffc550cc948
	READ of size 4 at 0x606000198b88 thread T0 (file:// Content)
		#0 0x7f94ab7d4e0f in IsEmpty /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTStringRepr.h:186:12
		#1 0x7f94ab7d4e0f in mozilla::PeerConnectionImpl::DTMFSendTimerCallback_m(nsITimer*, void*) /builds/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:3793
		#2 0x7f94a9b6a3d3 in nsTimerImpl::Fire(int) /builds/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp:701:7
		#3 0x7f94a9b39af9 in nsTimerEvent::Run() /builds/worker/workspace/build/src/xpcom/threads/TimerThread.cpp:286:11
		#4 0x7f94a9b4966e in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1033:14
		#5 0x7f94a9b653f0 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:508:10
		#6 0x7f94aa9d7984 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:125:5
		#7 0x7f94aa92e929 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
		#8 0x7f94aa92e929 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
		#9 0x7f94aa92e929 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
		#10 0x7f94b0d7f5ca in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:157:27
		#11 0x7f94b549418b in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:865:22
		#12 0x7f94aa92e929 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
		#13 0x7f94aa92e929 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
		#14 0x7f94aa92e929 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
		#15 0x7f94b5493b7d in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:691:34
		#16 0x4ee9f5 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
		#17 0x4ee9f5 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:280
		#18 0x7f94c6244ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
		#19 0x41e078 in _start (/home/coder/FirefoxBuilds/firefox/firefox+0x41e078)

	0x606000198b88 is located 40 bytes inside of 56-byte region [0x606000198b60,0x606000198b98)
	freed by thread T0 (file:// Content) here:
		#0 0x4bf212 in realloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:107:3
		#1 0x4f000d in moz_xrealloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:106:20
		#2 0x7f94a999c7dc in Realloc /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:214:12
		#3 0x7f94a999c7dc in nsTArrayInfallibleAllocator::ResultTypeProxy nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator>(unsigned long, unsigned long) /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray-inl.h:183
		#4 0x7f94ab7d413c in AppendElements<nsTArrayInfallibleAllocator> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1705:49
		#5 0x7f94ab7d413c in AppendElement<nsTArrayInfallibleAllocator> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1733
		#6 0x7f94ab7d413c in mozilla::PeerConnectionImpl::InsertDTMF(mozilla::TransceiverImpl&, nsTSubstring<char16_t> const&, unsigned int, unsigned int) /builds/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2453
		#7 0x7f94ad631a03 in InsertDTMF /builds/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h:422:10
		#8 0x7f94ad631a03 in mozilla::dom::PeerConnectionImplBinding::insertDTMF(JSContext*, JS::Handle<JSObject*>, mozilla::PeerConnectionImpl*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/PeerConnectionImplBinding.cpp:483
		#9 0x7f94aecc2ad7 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3042:13
		#10 0x7f94b5761041 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
		#11 0x7f94b5761041 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
		#12 0x7f94b57470a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
		#13 0x7f94b57470a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
		#14 0x7f94b5733810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
		#15 0x7f94b57614ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
		#16 0x7f94b57470a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
		#17 0x7f94b57470a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
		#18 0x7f94b5733810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
		#19 0x7f94b57614ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
		#20 0x7f94b57470a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
		#21 0x7f94b57470a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
		#22 0x7f94b5733810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
		#23 0x7f94b57614ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
		#24 0x7f94b5761fd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
		#25 0x7f94b625b40c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:3036:12
		#26 0x7f94ad79bae4 in mozilla::dom::RTCDTMFSenderJSImpl::InsertDTMF(nsTSubstring<char16_t> const&, unsigned int, unsigned int, mozilla::ErrorResult&, JSCompartment*) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/RTCDTMFSenderBinding.cpp:728:8
		#27 0x7f94ad8d2033 in InsertDTMF /builds/worker/workspace/build/src/obj-firefox/dom/bindings/RTCDTMFSenderBinding.cpp:891:17
		#28 0x7f94ad8d2033 in mozilla::dom::RTCDTMFSenderBinding::insertDTMF(JSContext*, JS::Handle<JSObject*>, mozilla::dom::RTCDTMFSender*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/RTCDTMFSenderBinding.cpp:64
		#29 0x7f94aecc2ad7 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3042:13
		#30 0x7f94b5761041 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
		#31 0x7f94b5761041 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
		#32 0x7f94b57470a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
		#33 0x7f94b57470a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
		#34 0x7f94b5733810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
		#35 0x7f94b57614ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
		#36 0x7f94b5761fd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
		#37 0x7f94b6544b52 in js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:176:12
		#38 0x7f94b64f261d in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /builds/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:359:23
		#39 0x7f94ab3a622b in xpc::JSXrayTraits::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&, js::Wrapper const&) /builds/worker/workspace/build/src/js/xpconnect/wrappers/XrayWrapper.h:282:33
		#40 0x7f94b6522d51 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /builds/worker/workspace/build/src/js/src/proxy/Proxy.cpp:511:21

	previously allocated by thread T0 (file:// Content) here:
		#0 0x4bee13 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3
		#1 0x4efe2d in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:84:17
		#2 0x7f94a999c71a in Malloc /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:211:46
		#3 0x7f94a999c71a in nsTArrayInfallibleAllocator::ResultTypeProxy nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator>(unsigned long, unsigned long) /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray-inl.h:136
		#4 0x7f94ab7d413c in AppendElements<nsTArrayInfallibleAllocator> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1705:49
		#5 0x7f94ab7d413c in AppendElement<nsTArrayInfallibleAllocator> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1733
		#6 0x7f94ab7d413c in mozilla::PeerConnectionImpl::InsertDTMF(mozilla::TransceiverImpl&, nsTSubstring<char16_t> const&, unsigned int, unsigned int) /builds/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2453
		#7 0x7f94ad631a03 in InsertDTMF /builds/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h:422:10
		#8 0x7f94ad631a03 in mozilla::dom::PeerConnectionImplBinding::insertDTMF(JSContext*, JS::Handle<JSObject*>, mozilla::PeerConnectionImpl*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/PeerConnectionImplBinding.cpp:483
		#9 0x7f94aecc2ad7 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3042:13
		#10 0x7f94b5761041 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
		#11 0x7f94b5761041 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
		#12 0x7f94b57470a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
		#13 0x7f94b57470a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
		#14 0x7f94b5733810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
		#15 0x7f94b57614ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
		#16 0x7f94b57470a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
		#17 0x7f94b57470a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
		#18 0x7f94b5733810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
		#19 0x7f94b57614ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
		#20 0x7f94b57470a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
		#21 0x7f94b57470a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
		#22 0x7f94b5733810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
		#23 0x7f94b57614ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
		#24 0x7f94b5761fd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
		#25 0x7f94b625b40c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:3036:12
		#26 0x7f94ad79bae4 in mozilla::dom::RTCDTMFSenderJSImpl::InsertDTMF(nsTSubstring<char16_t> const&, unsigned int, unsigned int, mozilla::ErrorResult&, JSCompartment*) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/RTCDTMFSenderBinding.cpp:728:8
		#27 0x7f94ad8d2033 in InsertDTMF /builds/worker/workspace/build/src/obj-firefox/dom/bindings/RTCDTMFSenderBinding.cpp:891:17
		#28 0x7f94ad8d2033 in mozilla::dom::RTCDTMFSenderBinding::insertDTMF(JSContext*, JS::Handle<JSObject*>, mozilla::dom::RTCDTMFSender*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/RTCDTMFSenderBinding.cpp:64
		#29 0x7f94aecc2ad7 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3042:13
		#30 0x7f94b5761041 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
		#31 0x7f94b5761041 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
		#32 0x7f94b57470a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
		#33 0x7f94b57470a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
		#34 0x7f94b5733810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
		#35 0x7f94b57614ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
		#36 0x7f94b5761fd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
		#37 0x7f94b593cd14 in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1238:14
		#38 0x7f94b5761041 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
		#39 0x7f94b5761041 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
		#40 0x7f94b5761fd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
		#41 0x7f94b625b40c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:3036:12

	SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTStringRepr.h:186:12 in IsEmpty
	Shadow bytes around the buggy address:
	  0x0c0c8002b120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	  0x0c0c8002b130: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
	  0x0c0c8002b140: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd
	  0x0c0c8002b150: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
	  0x0c0c8002b160: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
	=>0x0c0c8002b170: fd[fd]fd fa fa fa fa fa fd fd fd fd fd fd fd fa
	  0x0c0c8002b180: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
	  0x0c0c8002b190: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
	  0x0c0c8002b1a0: fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa
	  0x0c0c8002b1b0: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
	  0x0c0c8002b1c0: fd fd fd fd fd fd fd fd fa fa fa fa 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
	  Freed heap region:       fd
	  Stack left redzone:      f1
	  Stack mid redzone:       f2
	  Stack right redzone:     f3
	  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
	  Left alloca redzone:     ca
	  Right alloca redzone:    cb
	==9169==ABORTING
(Reporter)

Comment 1

a year ago
Corrected the fresh timer 2000 -> 200
Attachment #8934391 - Attachment is obsolete: true
Looks like it's the DTMF array that's gone and not the whole state object. Maybe less exploitable since no pointers involved? We do later delete a tone from it, though, so if you knew where to aim that could be bad. Waffling between sec-moderate and sec-high.
Group: core-security → media-core-security
(Reporter)

Comment 3

a year ago
It's an array of DTMF State object which does have pointers:

NS_IMETHODIMP
PeerConnectionImpl::InsertDTMF(TransceiverImpl& transceiver,
                               const nsAString& tones, uint32_t duration,
                               uint32_t interToneGap) {
  PC_AUTO_ENTER_API_CALL(false);

  // Check values passed in from PeerConnection.js
  MOZ_ASSERT(duration >= 40, "duration must be at least 40");
  MOZ_ASSERT(duration <= 6000, "duration must be at most 6000");
  MOZ_ASSERT(interToneGap >= 30, "interToneGap must be at least 30");

  JSErrorResult jrv;

  // TODO(bug 1401983): Move DTMF stuff to TransceiverImpl
  // Attempt to locate state for the DTMFSender
  DTMFState* state = nullptr;
  for (auto& dtmfState : mDTMFStates) {
    if (dtmfState.mTransceiver.get() == &transceiver) {
      state = &dtmfState;
      break;
    }
  }

  // No state yet, create a new one
  if (!state) {
    state = mDTMFStates.AppendElement();                <------------------------
    state->mPCObserver = mPCObserver;
    state->mTransceiver = &transceiver;
    state->mSendTimer = NS_NewTimer();
  }
  MOZ_ASSERT(state);

  state->mTones = tones;
  state->mDuration = duration;
  state->mInterToneGap = interToneGap;
  if (!state->mTones.IsEmpty()) {
    state->mSendTimer->InitWithNamedFuncCallback(DTMFSendTimerCallback_m, state, 0,
                                                 nsITimer::TYPE_ONE_SHOT,
                                                 "DTMFSendTimerCallback_m");
  }
  return NS_OK;
}
(Reporter)

Comment 4

a year ago
I got a PoC  UAF_DTMFSendTimerCallback_m_PoC_EIP_41414141.js with EIP control.

 	Steps to run:
	
	1. Run server side script UAF_DTMFSendTimerCallback_m_PoC_EIP_41414141.js in Node.js (node UAF_DTMFSendTimerCallback_m_PoC_EIP_41414141.js ).
	2. Enter http://localhost:12345 
        3. Firefox craches by executing code at location 0x41414141



Firefox version: 59.0a1 (2017-12-06) (32-bit)
OS: Windows 10 


(743c.5700): Access violation - code c0000005 (!!! second chance !!!)
eax=80000001 ebx=005ff0a0 ecx=b1b1b1b1 edx=00000004 esi=161cad3c edi=16196498
eip=41414141 esp=005ff068 ebp=005ff098 iopl=0         nv up ei pl zr na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
41414141 ??              ???
4:168> !analyze -v
*******************************************************************************
*                                                                             *
*                        Exception Analysis                                   *
*                                                                             *
*******************************************************************************

*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\WINDOWS\SysWOW64\nvwgf2um.dll - 
GetUrlPageData2 (WinHttp) failed: 12002.

FAULTING_IP: 
unknown!noop+0
41414141 ??              ???

EXCEPTION_RECORD:  (.exr -1)
ExceptionAddress: 41414141
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 00000008
   Parameter[1]: 41414141
Attempt to execute non-executable address 41414141

FAULTING_THREAD:  00005700

PROCESS_NAME:  firefox.exe

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

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

EXCEPTION_PARAMETER1:  00000008

EXCEPTION_PARAMETER2:  41414141

WRITE_ADDRESS:  41414141 

FOLLOWUP_IP: 
unknown!noop+0
41414141 ??              ???

FAILED_INSTRUCTION_ADDRESS: 
unknown!noop+0
41414141 ??              ???

NTGLOBALFLAG:  400

APPLICATION_VERIFIER_FLAGS:  0

APP:  firefox.exe

ANALYSIS_VERSION: 10.0.10240.9 x86fre

IP_ON_HEAP:  41414141
The fault address in not in any loaded module, please check your build's rebase
log at <releasedir>\bin\build_logs\timebuild\ntrebase.log for module which may
contain the address if it were loaded.

IP_IN_FREE_BLOCK: 41414141

BUGCHECK_STR:  SOFTWARE_NX_FAULT_INVALID

DEFAULT_BUCKET_ID:  SOFTWARE_NX_FAULT_INVALID

LAST_CONTROL_TRANSFER:  from 10fd179c to 41414141

STACK_TEXT:  
WARNING: Frame IP not in any known module. Following frames may be wrong.
005ff064 10fd179c 80000001 161cad68 161cad3c 0x41414141
005ff0a0 103c8c61 17e7cb20 16196498 12c83660 xul!mozilla::PeerConnectionImpl::DTMFSendTimerCallback_m+0xc4
005ff184 103c8b38 00000003 005ff720 03037ca0 xul!nsTimerImpl::Fire+0x11f
005ff1bc 103032ac 1617c048 03001230 03001220 xul!nsTimerEvent::Run+0x37
005ff728 1048bdcc 03037ca0 00000000 005ff753 xul!nsThread::ProcessNextEvent+0x294
005ff754 10dfdb8b 005ff8d0 005ff8d0 03003040 xul!mozilla::ipc::MessagePump::Run+0x75
005ff770 103d2d08 005ff8d0 314e32d9 07e08af0 xul!mozilla::ipc::MessagePumpForChildProcess::Run+0x58
005ff7a8 103d2cc8 03037ca0 00000002 03003000 xul!MessageLoop::RunHandler+0x1f
005ff7c8 106dc33e 07e08af0 005ff8d0 005ff7e8 xul!MessageLoop::Run+0x19
005ff7d8 106dc0ca 07e08af0 07e08af0 005ff800 xul!nsBaseAppShell::Run+0x34
005ff7e8 1223a419 07e08af0 005ff8d0 03001220 xul!nsAppShell::Run+0x26
005ff800 10dfdb49 005ff8d0 03001220 005ff848 xul!XRE_RunAppShell+0x30
005ff810 103d2d08 005ff8d0 314e3d39 00000013 xul!mozilla::ipc::MessagePumpForChildProcess::Run+0x16
005ff848 103d2cc8 03019800 00000001 005ff800 xul!MessageLoop::RunHandler+0x1f
005ff868 1223a2a2 030060f0 00000016 03003040 xul!MessageLoop::Run+0x19
005ff98c 1223cfea 005ff9b8 005ff9c0 009593ad xul!XRE_InitChildProcess+0x4bd
005ff998 009593ad 00000016 03003040 005ff9b8 xul!mozilla::BootstrapImpl::XRE_InitChildProcess+0x11
005ff9c0 00956813 03003040 02e62c08 754580e8 firefox!content_process_main+0x74
005ffd2c 009552b9 00000017 ffe5fbc8 02e63710 firefox!wmain+0x5313
005ffd74 75808654 007ac000 75808630 56693571 firefox!__scrt_common_main_seh+0xf8
005ffd88 77584a47 007ac000 548c7031 00000000 KERNEL32!BaseThreadInitThunk+0x24
005ffdd0 77584a17 ffffffff 775a9ecd 00000000 ntdll!__RtlUserThreadStart+0x2f
005ffde0 00000000 0095532f 007ac000 00000000 ntdll!_RtlUserThreadStart+0x1b


SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  unknown!noop+0

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: unknown

IMAGE_NAME:  unknown.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  0

STACK_COMMAND:  ~168s ; kb

BUCKET_ID:  SOFTWARE_NX_FAULT_INVALID_BAD_IP_unknown!noop+0

PRIMARY_PROBLEM_CLASS:  SOFTWARE_NX_FAULT_INVALID_BAD_IP_unknown!noop+0

FAILURE_PROBLEM_CLASS:  SOFTWARE_NX_FAULT_INVALID

FAILURE_EXCEPTION_CODE:  c0000005

FAILURE_IMAGE_NAME:  unknown.dll

FAILURE_FUNCTION_NAME:  noop

FAILURE_SYMBOL_NAME:  unknown.dll!noop

FAILURE_BUCKET_ID:  SOFTWARE_NX_FAULT_INVALID_c0000005_unknown.dll!noop

ANALYSIS_SOURCE:  UM

FAILURE_ID_HASH_STRING:  um:software_nx_fault_invalid_c0000005_unknown.dll!noop

FAILURE_ID_HASH:  {b7008989-cd4d-5e27-395d-2c076ddd0d3c}

Followup:     MachineOwner
---------
Nils: who should look at this bug?
Flags: needinfo?(drno)
Keywords: csectype-uaf, sec-critical
Flags: sec-bounty?
Nils, please check the code for 57/58/ESR, thanks
Rank: 3
status-firefox57: --- → ?
status-firefox58: --- → ?
status-firefox-esr52: --- → ?
Priority: -- → P1
(Assignee)

Comment 7

a year ago
A quick glance at the code in release indicates that this has been there for quite some time.
(Assignee)

Comment 8

a year ago
Yeah, we've been handing out non-temporary pointers into nsTArray's storage here since DTMF support landed (bug 1291715).
(Assignee)

Comment 9

a year ago
I'm taking a crack at this.
Assignee: nobody → docfaraday
(Assignee)

Comment 10

a year ago
Since this is going to be such a chemspill, should I bother trying to disguise the patch? Just using nsTArray<UniquePtr> is going to paint a pretty big bullseye on this problem. I could turn this into a "Use nsITimerCallback instead of a bare closure, which just looks like a good practice thing", which happens to require using nsTArray<RefPtr>, which will also fix this.
Flags: needinfo?(abillings)
(Assignee)

Updated

a year ago
status-firefox57: ? → affected
status-firefox58: ? → affected
status-firefox-esr52: ? → affected
(Assignee)

Updated

a year ago
Flags: needinfo?(drno)
(Assignee)

Comment 11

a year ago
Posted patch Store DTMFState in UniquePtr (obsolete) — Splinter Review
MozReview-Commit-ID: 2IlDknNhlAG
(In reply to Byron Campen [:bwc] from comment #10)
> Since this is going to be such a chemspill, should I bother trying to
> disguise the patch? Just using nsTArray<UniquePtr> is going to paint a
> pretty big bullseye on this problem. I could turn this into a "Use
> nsITimerCallback instead of a bare closure, which just looks like a good
> practice thing", which happens to require using nsTArray<RefPtr>, which will
> also fix this.

Where did we decide that this is going to be a chemspill (aka an emergency release)?

In any case, we should do our best to obfuscate what is being checked in to be the least obvious thing. :-)
Flags: needinfo?(abillings) → needinfo?(docfaraday)
(Assignee)

Comment 13

a year ago
We have an easy bug to hit with a proof of exploit on release and ESR, so we at least need to be careful on the timing. I will try to obfuscate some, and once the patch goes through review/approval here, I will be opening a cover bug and doing the try-pushes/checkin there (along with a re-review).
Flags: needinfo?(docfaraday)
(Assignee)

Comment 14

a year ago
MozReview-Commit-ID: 2IlDknNhlAG
(Assignee)

Updated

a year ago
Attachment #8935516 - Attachment is obsolete: true
(Assignee)

Comment 15

a year ago
Comment on attachment 8935767 [details] [diff] [review]
Use nsITimerCallback for DTMF timers

Approval Request Comment
[Feature/Bug causing the regression]:

   bug 1291715

[User impact if declined]:

   This flaw is an easily triggered (just add audio senders to make the DTMFState array grow) remote code execution exploit.

[Is this code covered by automated tests?]:

   Not very well, apparently. But it has very basic coverage.

[Has the fix been verified in Nightly?]:

   Not yet.

[Needs manual test from QE? If yes, steps to reproduce]: 

   No.

[List of other uplifts needed for the feature/fix]:

   I don't think this will need any uplifts. Backports will need to be created manually though.

[Is the change risky?]:

   Not particularly, at least it is far less risky than what we're doing now.

[Why is the change risky/not risky?]:

   For the most part, this is just changing how DTMFState is stored, using a different (safer) API for timers, and making these changes compile. There might be a small risk of leaks, because now there's a ref cycle between DTMFState and nsTimerImpl, but it looks like that cycle is broken when it needs to be.
[String changes made/needed]:

[String changes made/needed]:

   None.

Fix Landed on Version:

   Pending.

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

   An astute observer might notice that this bit of cleanup happens to fix a very nasty flaw, but it is somewhat obfuscated. If they do notice, it is a very easy exploit to construct. I plan to commit/try this in a cover bug once it passes review/approval here to further obfuscate.

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

   No.

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?

   Not yet, but I do not anticipate them being hard.

How likely is this patch to cause regressions; how much testing does it need?

   Not very likely.
Attachment #8935767 - Flags: sec-approval?
Attachment #8935767 - Flags: review?(drno)
Attachment #8935767 - Flags: approval-mozilla-release?
Attachment #8935767 - Flags: approval-mozilla-esr52?
Attachment #8935767 - Flags: approval-mozilla-beta?
Comment on attachment 8935767 [details] [diff] [review]
Use nsITimerCallback for DTMF timers

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

LGTM
Attachment #8935767 - Flags: review?(drno) → review+
Comment on attachment 8935767 [details] [diff] [review]
Use nsITimerCallback for DTMF timers

sec-approval+ but, ideally, I'd like to wait a few weeks to take this in since the end of year holidays are delaying the 58 Firefox release a bit.
Attachment #8935767 - Flags: sec-approval? → sec-approval+
tracking-firefox57: --- → ?
tracking-firefox58: --- → ?
tracking-firefox59: --- → ?
tracking-firefox-esr52: --- → ?
tracking-firefox58: ? → +
tracking-firefox59: ? → +
Duplicate of this bug: 1425222
tracking-firefox-esr52: ? → 58+
Comment on attachment 8935767 [details] [diff] [review]
Use nsITimerCallback for DTMF timers

Sec-crit, approved, Beta58+, ESR52.6+

Byrin, Nils, Gerry, RyanVM please check with Al/Dan on when they'd like this to be landed on m-b/m-esr52, see earlier comment from Al.
Flags: needinfo?(ryanvm)
Flags: needinfo?(gchang)
Flags: needinfo?(drno)
Flags: needinfo?(docfaraday)
Attachment #8935767 - Flags: approval-mozilla-esr52?
Attachment #8935767 - Flags: approval-mozilla-esr52+
Attachment #8935767 - Flags: approval-mozilla-beta?
Attachment #8935767 - Flags: approval-mozilla-beta+
(Assignee)

Comment 20

a year ago
When do we want to land this stuff? Should I mark this checkin-needed and let the security team land as desired? Or can you needinfo me when you're ready?
Flags: needinfo?(docfaraday) → needinfo?(abillings)
Land it now. :-)
Flags: needinfo?(abillings) → needinfo?(docfaraday)
Keywords: checkin-needed
(Assignee)

Comment 22

a year ago
Ok. I will now open up the cover bug, go through the motions on m-c, and get backports ready.
Flags: needinfo?(docfaraday)
(Assignee)

Comment 23

a year ago
Opened cover bug 1425901. Will try and land on m-c from there.
Flags: needinfo?(ryanvm)
Flags: needinfo?(gchang)
Flags: needinfo?(drno)
Keywords: checkin-needed
Bug 1425901 is on m-c now. I assume we'll do fresh approval requests in that bug as well.
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox59: affected → fixed
Resolution: --- → FIXED
(Assignee)

Comment 25

a year ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #24)
> Bug 1425901 is on m-c now. I assume we'll do fresh approval requests in that
> bug as well.

Should we do that in the cover bug, or handle the uplift here? My first inclination was to do the uplifts from here, using this bug's number on the backport patch. I could just request approval for beta/esr52 with an empty approval comment on the cover bug too, if that won't cause problems.
Flags: needinfo?(abillings)
Whiteboard: [fixed by bug 1425901]
(Assignee)

Comment 26

a year ago
Beta backport. Putting in here until I know which bug the uplift will happen in.
(Assignee)

Comment 27

a year ago
esr52 backport. Putting in here until I know which bug the uplift will happen in.
(Assignee)

Comment 28

a year ago
Possible dupe over in bug 1426185?
(In reply to Byron Campen [:bwc] from comment #28)
> Possible dupe over in bug 1426185?

I don't think so... that looks more like a shutdown-recursion problem with SpinEventLoopUntil<>
(Assignee)

Comment 30

a year ago
Looking for guidance on where to do the uplifts for this, since I haven't done this whole cover bug thing before.
Flags: needinfo?(dveditz)
(In reply to Byron Campen [:bwc] from comment #25)
> (In reply to Ryan VanderMeulen [:RyanVM] from comment #24)
> > Bug 1425901 is on m-c now. I assume we'll do fresh approval requests in that
> > bug as well.
> 
> Should we do that in the cover bug, or handle the uplift here? My first
> inclination was to do the uplifts from here, using this bug's number on the
> backport patch. I could just request approval for beta/esr52 with an empty
> approval comment on the cover bug too, if that won't cause problems.

I'd do the uplifts here.
Flags: needinfo?(abillings)
(Assignee)

Comment 32

a year ago
Got it.
Flags: needinfo?(dveditz)
(Assignee)

Comment 33

a year ago
uplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/bc166be85bb4f385de20cc87d9ba733f71245a6c
Bug 1423086: (esr52 backport) Use nsITimerCallback for DTMF timers. r=drno, a=ritu
status-firefox57: affected → wontfix
status-firefox58: affected → fixed
status-firefox-esr52: affected → fixed
Group: media-core-security → core-security-release
Flags: sec-bounty? → sec-bounty+
tracking-firefox57: ? → ---
Whiteboard: [fixed by bug 1425901] → [fixed by bug 1425901][adv-main58+][adv-esr52.6+]
Alias: CVE-2018-5091
Comment on attachment 8935767 [details] [diff] [review]
Use nsITimerCallback for DTMF timers

As m-r is 58 now, remove approval‑mozilla‑release flag.
Attachment #8935767 - Flags: approval-mozilla-release?
Flags: qe-verify+
Whiteboard: [fixed by bug 1425901][adv-main58+][adv-esr52.6+] → [fixed by bug 1425901][adv-main58+][adv-esr52.6+][post-critsmash-triage]
I managed to reproduce the issue on Firefox 59.0a1(2017-12-04) asan build, under Ubuntu 16.04x64.
The issue is no longer reproducible on Firefox 58.0 or on Firefox 59.0a1(2018-01-17).
Tests were performed under Ubuntu 16.04x64, Windows 10x64 and under macOS 10.13.2.
Status: RESOLVED → VERIFIED
status-firefox58: fixed → verified
status-firefox59: fixed → verified
Flags: qe-verify+
Also verified on Firefox 52.6.0ESR.
status-firefox-esr52: fixed → verified
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.