Heap-use-after-free in mozilla::dom::HTMLMediaElement::NotifyDecoderPrincipalChanged

VERIFIED FIXED in Firefox 48

Status

()

defect
P1
normal
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: inferno, Assigned: pehrsons)

Tracking

({csectype-uaf, regression, sec-high})

Trunk
mozilla49
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +
qe-verify +

Firefox Tracking Flags

(firefox47 unaffected, firefox48+ verified, firefox49+ verified, firefox-esr45 unaffected)

Details

Attachments

(2 attachments)

Reporter

Description

3 years ago
Posted file Testcase
Load testcase test.html in multiple tabs (4-5) and wait for 30-60 sec to see this crash in one of the tabs.

==7931==ERROR: AddressSanitizer: heap-use-after-free on address 0x6080008191e8 at pc 0x7fbee23df088 bp 0x7fff819e94b0 sp 0x7fff819e94a8
READ of size 8 at 0x6080008191e8 thread T0 (Web Content)
    #0 0x7fbee23df087 in mozilla::dom::HTMLMediaElement::NotifyDecoderPrincipalChanged() /build/firefox/src/dom/html/HTMLMediaElement.cpp:4403:5
    #1 0x7fbee23cb2c5 in mozilla::dom::HTMLMediaElement::FinishDecoderSetup(mozilla::MediaDecoder*, mozilla::MediaResource*, nsIStreamListener**) /build/firefox/src/dom/html/HTMLMediaElement.cpp:3033:3
    #2 0x7fbee23cac57 in mozilla::dom::HTMLMediaElement::InitializeDecoderAsClone(mozilla::MediaDecoder*) /build/firefox/src/dom/html/HTMLMediaElement.cpp:2953:10
    #3 0x7fbee23c5fef in mozilla::dom::HTMLMediaElement::LoadResource() /build/firefox/src/dom/html/HTMLMediaElement.cpp:1261:19
    #4 0x7fbee23c25fe in mozilla::dom::HTMLMediaElement::LoadFromSourceChildren() /build/firefox/src/dom/html/HTMLMediaElement.cpp:1107:9
    #5 0x7fbee23c41b5 in mozilla::dom::HTMLMediaElement::SelectResource() /build/firefox/src/dom/html/HTMLMediaElement.cpp:969:5
    #6 0x7fbee23c38ed in mozilla::dom::HTMLMediaElement::SelectResourceWrapper() /build/firefox/src/dom/html/HTMLMediaElement.cpp:905:3
    #7 0x7fbee23ff790 in applyImpl<mozilla::dom::HTMLMediaElement, void (mozilla::dom::HTMLMediaElement::*)()> /build/firefox/src/objdir-ff-asan/dist/include/nsThreadUtils.h:671:12
    #8 0x7fbee23ff790 in apply<mozilla::dom::HTMLMediaElement, void (mozilla::dom::HTMLMediaElement::*)()> /build/firefox/src/objdir-ff-asan/dist/include/nsThreadUtils.h:677
    #9 0x7fbee23ff790 in nsRunnableMethodImpl<void (mozilla::dom::HTMLMediaElement::*)(), true>::Run() /build/firefox/src/objdir-ff-asan/dist/include/nsThreadUtils.h:705
    #10 0x7fbee23f81cf in mozilla::dom::nsSyncSection::Run() /build/firefox/src/dom/html/HTMLMediaElement.cpp:821:5
    #11 0x7fbedd2518e7 in mozilla::CycleCollectedJSRuntime::ProcessStableStateQueue() /build/firefox/src/xpcom/base/CycleCollectedJSRuntime.cpp:1327:5
    #12 0x7fbedec9c121 in XPCJSRuntime::AfterProcessTask(unsigned int) /build/firefox/src/js/xpconnect/src/XPCJSRuntime.cpp:3719:5
    #13 0x7fbedd394fec in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:1009:5
    #14 0x7fbedd414b8c in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #15 0x7fbede16472f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:98:21
    #16 0x7fbede0daa71 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:230:3
    #17 0x7fbede0daa71 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:223
    #18 0x7fbede0daa71 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:203
    #19 0x7fbee394ebff in nsBaseAppShell::Run() /build/firefox/src/widget/nsBaseAppShell.cpp:156:3
    #20 0x7fbee5a52053 in XRE_RunAppShell /build/firefox/src/toolkit/xre/nsEmbedFunctions.cpp:801:12
    #21 0x7fbede0daa71 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:230:3
    #22 0x7fbede0daa71 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:223
    #23 0x7fbede0daa71 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:203
    #24 0x7fbee5a515f9 in XRE_InitChildProcess /build/firefox/src/toolkit/xre/nsEmbedFunctions.cpp:637:7
    #25 0x4eaade in content_process_main(int, char**) /build/firefox/src/ipc/app/../contentproc/plugin-container.cpp:237:19
    #26 0x7fbeda4fcec4 in __libc_start_main
    #27 0x41dbc6 in _start

0x6080008191e8 is located 72 bytes inside of 88-byte region [0x6080008191a0,0x6080008191f8)
freed by thread T0 (Web Content) here:
    #0 0x4b9600 in __interceptor_free _asan_rtl_
    #1 0x7fbedd26c025 in SnowWhiteKiller::~SnowWhiteKiller() /build/firefox/src/xpcom/base/nsCycleCollector.cpp:2675:9
    #2 0x7fbedd26bb5a in nsCycleCollector::FreeSnowWhite(bool) /build/firefox/src/xpcom/base/nsCycleCollector.cpp:2849:3
    #3 0x7fbedecb345e in AsyncFreeSnowWhite::Run() /build/firefox/src/js/xpconnect/src/XPCJSRuntime.cpp:155:34
    #4 0x7fbedd394b35 in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:994:7
    #5 0x7fbedd414b8c in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #6 0x7fbede16472f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:98:21
    #7 0x7fbede0daa71 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:230:3
    #8 0x7fbede0daa71 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:223
    #9 0x7fbede0daa71 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:203
    #10 0x7fbee394ebff in nsBaseAppShell::Run() /build/firefox/src/widget/nsBaseAppShell.cpp:156:3
    #11 0x7fbee5a52053 in XRE_RunAppShell /build/firefox/src/toolkit/xre/nsEmbedFunctions.cpp:801:12
    #12 0x7fbede0daa71 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:230:3
    #13 0x7fbede0daa71 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:223
    #14 0x7fbede0daa71 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:203
    #15 0x7fbee5a515f9 in XRE_InitChildProcess /build/firefox/src/toolkit/xre/nsEmbedFunctions.cpp:637:7
    #16 0x4eaade in content_process_main(int, char**) /build/firefox/src/ipc/app/../contentproc/plugin-container.cpp:237:19
    #17 0x7fbeda4fcec4 in __libc_start_main

previously allocated by thread T0 (Web Content) here:
    #0 0x4b98f8 in __interceptor_malloc _asan_rtl_
    #1 0x4eacfd in moz_xmalloc /build/firefox/src/memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7fbee23f6c49 in operator new /build/firefox/src/objdir-ff-asan/dist/include/mozilla/mozalloc.h:186:12
    #3 0x7fbee23f6c49 in mozilla::dom::HTMLMediaElement::CaptureStreamTrackSourceGetter::GetMediaStreamTrackSource(int) /build/firefox/src/dom/html/HTMLMediaElement.cpp:1981
    #4 0x7fbee23d4bbf in mozilla::dom::HTMLMediaElement::CaptureStreamInternal(bool, mozilla::MediaStreamGraph*) /build/firefox/src/dom/html/HTMLMediaElement.cpp:2047:11
    #5 0x7fbee23d5279 in mozilla::dom::HTMLMediaElement::MozCaptureStreamUntilEnded(mozilla::ErrorResult&, mozilla::MediaStreamGraph*) /build/firefox/src/dom/html/HTMLMediaElement.cpp:2074:35
    #6 0x7fbee1beb49f in mozilla::dom::HTMLMediaElementBinding::mozCaptureStreamUntilEnded(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLMediaElement*, JSJitMethodCallArgs const&) /build/firefox/src/objdir-ff-asan/dom/bindings/HTMLMediaElementBinding.cpp:1975:55
    #7 0x7fbee1d9849a in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /build/firefox/src/dom/bindings/BindingUtils.cpp:2778:13
    #8 0x7fbee7d8a305 in CallJSNative /build/firefox/src/js/src/jscntxtinlines.h:235:15
    #9 0x7fbee7d8a305 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /build/firefox/src/js/src/vm/Interpreter.cpp:468
    #10 0x7fbee7d7302c in CallFromStack /build/firefox/src/js/src/vm/Interpreter.cpp:531:12
    #11 0x7fbee7d7302c in Interpret(JSContext*, js::RunState&) /build/firefox/src/js/src/vm/Interpreter.cpp:2831
    #12 0x7fbee7d57586 in js::RunScript(JSContext*, js::RunState&) /build/firefox/src/js/src/vm/Interpreter.cpp:426:12
    #13 0x7fbee7d8c4c8 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /build/firefox/src/js/src/vm/Interpreter.cpp:704:15
    #14 0x7fbee7711d1b in EvalKernel(JSContext*, JS::Handle<JS::Value>, EvalType, js::AbstractFramePtr, JS::Handle<JSObject*>, unsigned char*, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/builtin/Eval.cpp:327:12
    #15 0x7fbee77128be in js::DirectEval(JSContext*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/builtin/Eval.cpp:439:12
    #16 0x7fbee7d61e13 in Interpret(JSContext*, js::RunState&) /build/firefox/src/js/src/vm/Interpreter.cpp:2746:14
    #17 0x7fbee7d57586 in js::RunScript(JSContext*, js::RunState&) /build/firefox/src/js/src/vm/Interpreter.cpp:426:12
    #18 0x7fbee7d8a51b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /build/firefox/src/js/src/vm/Interpreter.cpp:498:15
    #19 0x7fbee7d3d581 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/vm/Interpreter.cpp:544:10
    #20 0x7fbee78aee40 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/jsapi.cpp:2919:12
    #21 0x7fbee1a04ba9 in mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /build/firefox/src/objdir-ff-asan/dom/bindings/FunctionBinding.cpp:36:8
    #22 0x7fbedfd2b806 in Call<nsCOMPtr<nsISupports> > /build/firefox/src/objdir-ff-asan/dist/include/mozilla/dom/FunctionBinding.h:64:12
    #23 0x7fbedfd2b806 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:11961
    #24 0x7fbedfd09f7b in nsGlobalWindow::RunTimeout(nsTimeout*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:12210:32
    #25 0x7fbedfca8281 in nsGlobalWindow::TimerCallback(nsITimer*, void*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:12456:3
    #26 0x7fbedd3b0345 in nsTimerImpl::Fire() /build/firefox/src/xpcom/threads/nsTimerImpl.cpp:524:7
    #27 0x7fbedd3881fc in nsTimerEvent::Run() /build/firefox/src/xpcom/threads/TimerThread.cpp:286:3
    #28 0x7fbedd394b35 in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:994:7
    #29 0x7fbedd414b8c in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #30 0x7fbede16472f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:98:21
    #31 0x7fbede0daa71 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:230:3
    #32 0x7fbede0daa71 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:223
    #33 0x7fbede0daa71 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:203
    #34 0x7fbee394ebff in nsBaseAppShell::Run() /build/firefox/src/widget/nsBaseAppShell.cpp:156:3
    #35 0x7fbee5a52053 in XRE_RunAppShell /build/firefox/src/toolkit/xre/nsEmbedFunctions.cpp:801:12

SUMMARY: AddressSanitizer: heap-use-after-free (/build/firefox/src/objdir-ff-asan/dist/bin/libxul.so+0x6f13087)
Shadow bytes around the buggy address:
  0x0c10800fb1e0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c10800fb1f0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c10800fb200: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c10800fb210: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c10800fb220: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
=>0x0c10800fb230: fa fa fa fa fd fd fd fd fd fd fd fd fd[fd]fd fa
  0x0c10800fb240: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c10800fb250: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c10800fb260: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c10800fb270: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c10800fb280: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
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
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==7931==ABORTING
Group: core-security → media-core-security
Flags: needinfo?(ajones)
Component: Audio/Video → Audio/Video: Playback
Priority: -- → P1
Jesup - how did you determine that this is playback rather than capture?
Flags: needinfo?(ajones) → needinfo?(rjesup)
While this uses mozCaptureStreamUntilEnded in the test, the last several functions in the crash are
SelectResourceWrapper()/.../InitializeDecoderAsClone()/FinishDecoderSetup()/NotifyDecoderPrincipalChanged()

I'd guess this is related to the teardown/recreation on seeks (note the currentTime changes mixed with play and pause).

I should also note that the memory referenced was allocated here:
HTMLMediaElement::CaptureStreamTrackSourceGetter::GetMediaStreamTrackSource()
(Pehrsons? could you glance at this, since it was touched by the cloning landing?)


Hmmm.  I'll note that the script uses V.mozCaptureStreamUntilEnded(), but doesn't retain a reference to it - thus allowing it to be GC'd (which may be the source of the free above) - perhaps the problem is not ferreting out the reference to it somewhere when that happens.  (pehrsons, padenot, jya?)
Flags: needinfo?(rjesup) → needinfo?(pehrsons)
Assignee

Comment 3

3 years ago
I could repro this in the debugger.

We have a CaptureStreamTrackSource, because the media element is captured to a stream, that registers itself as a DecoderPrincipalChangeObserver.

It would unregister itself in the destructor.

However, the garbage collector will sometimes clean up the CaptureStreamTrackSource before the media element, so once the destructor is reached, mElement is null and we cannot deregister the observer.

After this happened, the media element tries to notify the (destructed) CaptureStreamTrackSource that the decoder's principal changed.

I'll fix the GC macro to call a Destroy() method and double check that no other MediaStreamTrackSources are doing the same thing.
Assignee: nobody → pehrsons
Blocks: 1208371
Status: NEW → ASSIGNED
Component: Audio/Video: Playback → Audio/Video: MediaStreamGraph
Flags: needinfo?(pehrsons)
Attachment #8747014 - Flags: review?(rjesup) → review+
Assignee

Comment 5

3 years ago
Comment on attachment 8747014 [details] [diff] [review]
Move CaptureStreamTrackSource cleanup to Destroy method

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
I can't see how an exploit could be made from this (i.e., how to jump to an address of the exploiter's choice), other than crashing somewhat at will.

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. A comment mentions issues related to when the garbage collector and destructors run.

Which older supported branches are affected by this flaw?
Aurora 48.

If not all supported branches, which bug introduced the flaw?
Bug 1208371.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Simple uplift, very low risk.

How likely is this patch to cause regressions; how much testing does it need?
Very unlikely.
Attachment #8747014 - Flags: sec-approval?
sec-approval+ for trunk. 

Please nominate an Aurora patch as well so we don't ship this issue.
Attachment #8747014 - Flags: sec-approval? → sec-approval+
Assignee

Updated

3 years ago
Keywords: checkin-needed
Assignee

Comment 7

3 years ago
Comment on attachment 8747014 [details] [diff] [review]
Move CaptureStreamTrackSource cleanup to Destroy method

Approval Request Comment
[Feature/regressing bug #]: Bug 1208371
[User impact if declined]: Security issue.
[Describe test coverage new/current, TreeHerder]: Verified manually.
[Risks and why]: Low risk. Simple change.
[String/UUID change made/needed]: None.
Attachment #8747014 - Flags: approval-mozilla-aurora?
Attachment #8747014 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/a59c7993e374
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Group: media-core-security → core-security-release
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Keywords: regression
Group: core-security-release
Reproduced the crash on Nightly 2016-05-01 Win 7.
Verified fixed FX 48b2, 49.0a2 (2016-06-23).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.