787831 - (CVE-2013-0761) Heap-use-after-free in mozilla::TrackUnionStream::EndTrack

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Abhishek Arya]

Attachment: Testcase

Reproduces on trunk

MediaStreamGraphImpl::FinishStream
MediaStreamGraphImpl::FinishStream
nsBuiltinDecoderStateMachine::RunStateMachine queuing nsBuiltinDecoder::PlaybackEnded
=================================================================
==26602== ERROR: AddressSanitizer heap-use-after-free on address 0x7f9ea7933198 at pc 0x7f9ec9a0034e bp 0x7f9e8f5ea310 sp 0x7f9e8f5ea308
READ of size 1 at 0x7f9ea7933198 thread T14
    #0 0x7f9ec9a0034d in mozilla::StreamBuffer::Track::IsEnded() const src/content/media/StreamBuffer.h:125
    #1 0x7f9ec9a03db5 in mozilla::TrackUnionStream::EndTrack(unsigned int) src/content/media/TrackUnionStream.h:148
    #2 0x7f9ec99ff3e8 in mozilla::TrackUnionStream::ProduceOutput(long, long) src/content/media/TrackUnionStream.h:83
    #3 0x7f9ec99bd827 in mozilla::MediaStreamGraphImpl::RunThread() src/content/media/MediaStreamGraph.cpp:1395
    #4 0x7f9ec9a1b272 in mozilla::(anonymous namespace)::MediaStreamGraphThreadRunnable::Run() src/content/media/MediaStreamGraph.cpp:1510
    #5 0x7f9ecfa2949e in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:624
    #6 0x7f9ecf6ca877 in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
    #7 0x7f9ecfa213c5 in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:257
    #8 0x7f9edea519c3 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:156
    #9 0x4c6f8b in __asan::AsanThread::ThreadStart() ??:0
0x7f9ea7933198 is located 24 bytes inside of 32-byte region [0x7f9ea7933180,0x7f9ea79331a0)
freed by thread T14 here:
    #0 0x4c3e30 in free ??:0
    #1 0x7f9edc76d572 in moz_free src/memory/mozalloc/mozalloc.cpp:51
    #2 0x7f9ec99f668e in operator delete(void*) src/../../dist/include/mozilla/mozalloc.h:224
    #3 0x7f9ec99f64c2 in ~nsAutoPtr src/../../dist/include/nsAutoPtr.h:70
    #4 0x7f9ec99f63b2 in nsTArrayElementTraits<nsAutoPtr<mozilla::StreamBuffer::Track> >::Destruct(nsAutoPtr<mozilla::StreamBuffer::Track>*) src/../../dist/include/nsTArray.h:348
    #5 0x7f9ec99f60b3 in nsTArray<nsAutoPtr<mozilla::StreamBuffer::Track>, nsTArrayDefaultAllocator>::DestructRange(unsigned int, unsigned int) src/../../dist/include/nsTArray.h:1213
    #6 0x7f9ec99f5b5c in nsTArray<nsAutoPtr<mozilla::StreamBuffer::Track>, nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int) src/../../dist/include/nsTArray.h:933
    #7 0x7f9ec9b0dc58 in nsTArray<nsAutoPtr<mozilla::StreamBuffer::Track>, nsTArrayDefaultAllocator>::RemoveElementAt(unsigned int) src/../../dist/include/nsTArray.h:939
    #8 0x7f9ec9b0d630 in mozilla::StreamBuffer::ForgetUpTo(long) src/content/media/StreamBuffer.cpp:51
    #9 0x7f9ec99cff93 in mozilla::MediaStream::AdvanceTimeVaryingValuesToCurrentTime(long, long) src/content/media/MediaStreamGraph.h:385
    #10 0x7f9ec99a441b in mozilla::MediaStreamGraphImpl::UpdateCurrentTime() src/content/media/MediaStreamGraph.cpp:816
    #11 0x7f9ec99bc7da in mozilla::MediaStreamGraphImpl::RunThread() src/content/media/MediaStreamGraph.cpp:1352
    #12 0x7f9ec9a1b272 in mozilla::(anonymous namespace)::MediaStreamGraphThreadRunnable::Run() src/content/media/MediaStreamGraph.cpp:1510
    #13 0x7f9ecfa2949e in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:624
    #14 0x7f9ecf6ca877 in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
    #15 0x7f9ecfa213c5 in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:257
    #16 0x7f9edea519c3 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:156
    #17 0x4c6f8b in __asan::AsanThread::ThreadStart() ??:0
previously allocated by thread T14 here:
    #0 0x4c3ef0 in __interceptor_malloc ??:0
    #1 0x7f9edc76d6c6 in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:57
    #2 0x7f9ec999de8d in operator new(unsigned long) src/../../dist/include/mozilla/mozalloc.h:200
    #3 0x7f9ec9a0343f in mozilla::TrackUnionStream::AddTrack(mozilla::MediaInputPort*, mozilla::StreamBuffer::Track*, long) src/content/media/TrackUnionStream.h:133
    #4 0x7f9ec99fefd2 in mozilla::TrackUnionStream::ProduceOutput(long, long) src/content/media/TrackUnionStream.h:74
    #5 0x7f9ec99bd827 in mozilla::MediaStreamGraphImpl::RunThread() src/content/media/MediaStreamGraph.cpp:1395
    #6 0x7f9ec9a1b272 in mozilla::(anonymous namespace)::MediaStreamGraphThreadRunnable::Run() src/content/media/MediaStreamGraph.cpp:1510
    #7 0x7f9ecfa2949e in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:624
    #8 0x7f9ecf6ca877 in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
    #9 0x7f9ecfa213c5 in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:257
    #10 0x7f9edea519c3 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:156
    #11 0x4c6f8b in __asan::AsanThread::ThreadStart() ??:0
Thread T14 created by T0 here:
    #0 0x4c0284 in pthread_create ??:0
    #1 0x7f9edea42ac9 in _PR_CreateThread src/nsprpub/pr/src/pthreads/ptthread.c:393
    #2 0x7f9edea40bc2 in PR_CreateThread src/nsprpub/pr/src/pthreads/ptthread.c:476
    #3 0x7f9ecfa244e9 in nsThread::Init() src/xpcom/threads/nsThread.cpp:323
    #4 0x7f9ecfa3b463 in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) src/xpcom/threads/nsThreadManager.cpp:215
    #5 0x7f9ecf6c7d2a in NS_NewThread_P(nsIThread**, nsIRunnable*, unsigned int) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:49
    #6 0x7f9ec99c0938 in mozilla::MediaStreamGraphImpl::RunInStableState() src/content/media/MediaStreamGraph.cpp:1617
    #7 0x7f9ec9a1ad14 in mozilla::(anonymous namespace)::MediaStreamGraphStableStateRunnable::Run() src/content/media/MediaStreamGraph.cpp:1544
    #8 0x7f9ecd95c352 in nsBaseAppShell::RunSyncSectionsInternal(bool, unsigned int) src/widget/xpwidgets/nsBaseAppShell.cpp:352
    #9 0x7f9ecd95833d in nsBaseAppShell::RunSyncSections(bool, unsigned int) src/widget/xpwidgets/nsBaseAppShell.h:89
    #10 0x7f9ecd95dcd0 in nsBaseAppShell::AfterProcessNextEvent(nsIThreadInternal*, unsigned int) src/widget/xpwidgets/nsBaseAppShell.cpp:408
    #11 0x7f9ecd95dd84 in non-virtual thunk to nsBaseAppShell::AfterProcessNextEvent(nsIThreadInternal*, unsigned int) src/gfx/cairo/cairo/src/cairo-surface-subsurface.c:0
    #12 0x7f9ecfa298b8 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:637
    #13 0x7f9ecf6ca877 in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
    #14 0x7f9ece493515 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82
    #15 0x7f9ecfcd4099 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:208
    #16 0x7f9ecfcd3ee2 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:201
    #17 0x7f9ecfcd3dc7 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:175
    #18 0x7f9ecd95864e in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
    #19 0x7f9ecc5be178 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:273
    #20 0x7f9ec2dd1450 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3835
    #21 0x7f9ec2dd76c4 in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3912
    #22 0x7f9ec2dda78e in XRE_main src/toolkit/xre/nsAppRunner.cpp:3988
    #23 0x40c5bb in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174
    #24 0x409e1f in main src/browser/app/nsBrowserApp.cpp:279
    #25 0x7f9edf8cfc4d in ?? ??:0
Shadow byte and word:
  0x1ff3d4f26633: fd
  0x1ff3d4f26630: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1ff3d4f26610: 00 00 02 fb fb fb fb fb
  0x1ff3d4f26618: fb fb fb fb fb fb fb fb
  0x1ff3d4f26620: fa fa fa fa fa fa fa fa
  0x1ff3d4f26628: fa fa fa fa fa fa fa fa
=>0x1ff3d4f26630: fd fd fd fd fd fd fd fd
  0x1ff3d4f26638: fd fd fd fd fd fd fd fd
  0x1ff3d4f26640: fa fa fa fa fa fa fa fa
  0x1ff3d4f26648: fa fa fa fa fa fa fa fa
  0x1ff3d4f26650: fd fd fd fd fd fd fd fd
Stats: 282M malloced (279M for red zones) by 392075 calls
Stats: 43M realloced by 22062 calls
Stats: 239M freed by 253504 calls
Stats: 98M really freed by 176236 calls
Stats: 488M (125019 full pages) mmaped in 122 calls
  mmaps   by size class: 8:245745; 9:40955; 10:16380; 11:14329; 12:3072; 13:1536; 14:1280; 15:256; 16:448; 17:1248; 18:144; 19:152; 20:16;
  mallocs by size class: 8:296665; 9:50967; 10:17702; 11:17532; 12:2750; 13:1931; 14:1873; 15:343; 16:557; 17:1418; 18:169; 19:151; 20:17;
  frees   by size class: 8:177303; 9:40842; 10:13985; 11:14149; 12:1762; 13:1413; 14:1646; 15:291; 16:497; 17:1398; 18:57; 19:147; 20:14;
  rfrees  by size class: 8:130433; 9:23363; 10:9030; 11:9935; 12:870; 13:705; 14:725; 15:168; 16:347; 17:626; 18:28; 19:5; 20:1;
Stats: malloc large: 1755 small slow: 2053
==26602== ABORTING

Comment 1

[Andrew McCreight [:mccr8]]

The use-after-free stack has TrackUnionStream in it, so maybe it is related to bug 779715, which landed in 17.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This looks like an array out of bounds error accessing elements of mTrackMap?

However, there are two callers of EndTrack(i) and both of them pretty clearly ensure that i < mTrackMap.Length(). Maybe this got fixed?

Comment 3

[Abhishek Arya]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> This looks like an array out of bounds error accessing elements of mTrackMap?
> 
> However, there are two callers of EndTrack(i) and both of them pretty
> clearly ensure that i < mTrackMap.Length(). Maybe this got fixed?

This still reproduces easily on trunk using a single firefox instance and an asanified linux build.

Comment 4

[Mats Palmgren (inactive)]

I can reproduce in an up-to-date mozilla-central debug+asan Linux64 build.
Before the crash I get this on the console:
WARNING: Can't add a range if the end is older that the start.: file content/html/content/src/nsTimeRanges.cpp, line 61
MediaStreamGraphImpl::FinishStream
nsBuiltinDecoderStateMachine::RunStateMachine queuing nsBuiltinDecoder::PlaybackEnded

Comment 5

[Mats Palmgren (inactive)]

Attachment: stack

In a non-asan debug build I get this crash. Possibly unrelated though.

STR: load the url in 30 or so tabs; in a tab context menu: Reload all tabs,
resize the window, close a few tabs. Repeat.

Comment 6

[Mats Palmgren (inactive)]

Attachment: another crash

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This fixes it for me in my asan build.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 677347 [details] [diff] [review]
fix

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

::: content/media/TrackUnionStream.h
@@ +170,5 @@
>                       bool* aOutputTrackFinished)
>    {
>      TrackMapEntry* map = &mTrackMap[aMapIndex];
> +    StreamBuffer::Track* outputTrack = mBuffer.FindTrack(map->mOutputTrackID);
> +    NS_ASSERTION(outputTrack && !outputTrack->IsEnded(), "Can't copy to ended track");

Do we need runtime protection here against misuse not caught in testing or for opt nightly/aurora/beta builds, or is nullptr crashing on the next line ok/preferred?

And non-important, isn't MOZ_ASSERT(blah, "what went wrong") the new hotness?  :-)

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think nullptr crashing is adequate here.

I suppose MOZ_ASSERT would be fine here too, since we're going to crash anyway. I'll switch it to that.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/519848fd5eaa

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 677347 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): TrackUnionStream landing
User impact if declined: possible crash/security bug
Testing completed (on m-c, etc.): none
Risk to taking this patch (and alternatives if risky): Low risk. This feature is so far not really used.
String or UUID changes made by this patch: None.

Comment 12

[Alex Keybl [:akeybl]]

Comment on attachment 677347 [details] [diff] [review]
fix

[Triage Comment]
Given this is most prevalently used for WebRTC, we can be fairly sure that regressions will be limited to that pref'd off feature. Approving for Aurora 18.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/519848fd5eaa

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2a2c5e00b5f6

Comment 15

[Daniel Veditz [:dveditz]]

This should affect the upcoming ESR-17, right? We'll want the fix there even if it's too late to safely land this fix in 17.0

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think this bug is in 17.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Oh, it is, never mind.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 677347 [details] [diff] [review]
fix

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Security fix
Fix Landed on Version: 18, 19
Risk to taking this patch (and alternatives if risky): low risk, been on trunk a while
String or UUID changes made by this patch: none

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr17/rev/b216a3a825a1

Comment 20

[Daniel Veditz [:dveditz]]

What bad object is being read? something with pointers or a vtable that can be abused or just data about the track that will lead to garbage?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The latter.