Last Comment Bug 787831 - (CVE-2013-0761) Heap-use-after-free in mozilla::TrackUnionStream::EndTrack
(CVE-2013-0761)
: Heap-use-after-free in mozilla::TrackUnionStream::EndTrack
Status: RESOLVED FIXED
[asan][adv-main18+][adv-esr17+]
: crash, regression, sec-moderate, testcase
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86_64 All
: -- critical (vote)
: mozilla19
Assigned To: Robert O'Callahan (:roc) (email my personal email if necessary)
:
: Maire Reavy [:mreavy]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-02 13:31 PDT by Abhishek Arya
Modified: 2013-04-30 18:40 PDT (History)
7 users (show)
dveditz: sec‑bounty-
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
-
fixed
fixed
unaffected
18+
fixed


Attachments
Testcase (322.91 KB, application/x-zip-compressed)
2012-09-02 13:31 PDT, Abhishek Arya
no flags Details
stack (13.38 KB, text/plain)
2012-09-26 07:59 PDT, Mats Palmgren (:mats)
no flags Details
another crash (6.38 KB, text/plain)
2012-09-26 08:22 PDT, Mats Palmgren (:mats)
no flags Details
fix (7.93 KB, patch)
2012-11-01 04:22 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
rjesup: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑esr17+
Details | Diff | Splinter Review

Description Abhishek Arya 2012-09-02 13:31:33 PDT
Created attachment 657709 [details]
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] 2012-09-05 11:14:19 PDT
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) 2012-09-26 05:14:26 PDT
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 2012-09-26 07:38:49 PDT
(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 (:mats) 2012-09-26 07:51:05 PDT
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 (:mats) 2012-09-26 07:59:45 PDT
Created attachment 664965 [details]
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 (:mats) 2012-09-26 08:22:35 PDT
Created attachment 664982 [details]
another crash
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-11-01 04:22:13 PDT
Created attachment 677347 [details] [diff] [review]
fix

This fixes it for me in my asan build.
Comment 8 Randell Jesup [:jesup] 2012-11-01 09:59:02 PDT
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) 2012-11-01 13:52:03 PDT
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) 2012-11-02 06:12:29 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/519848fd5eaa
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-11-02 06:15:17 PDT
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] 2012-11-02 15:27:34 PDT
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] 2012-11-02 17:30:55 PDT
https://hg.mozilla.org/mozilla-central/rev/519848fd5eaa
Comment 14 Ryan VanderMeulen [:RyanVM] 2012-11-03 08:57:23 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/2a2c5e00b5f6
Comment 15 Daniel Veditz [:dveditz] 2012-11-08 16:48:38 PST
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) 2012-11-09 14:19:05 PST
I don't think this bug is in 17.
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-11-09 14:19:32 PST
Oh, it is, never mind.
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-11-09 14:20:41 PST
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] 2012-12-01 20:41:41 PST
https://hg.mozilla.org/releases/mozilla-esr17/rev/b216a3a825a1
Comment 20 Daniel Veditz [:dveditz] 2012-12-03 15:37:55 PST
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) 2012-12-03 20:42:33 PST
The latter.

Note You need to log in before you can comment on or make changes to this bug.