Closed Bug 787831 (CVE-2013-0761) Opened 12 years ago Closed 12 years ago

Heap-use-after-free in mozilla::TrackUnionStream::EndTrack

Categories

(Core :: Audio/Video, defect)

x86_64
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox17 --- wontfix
firefox18 - fixed
firefox19 --- fixed
firefox-esr10 --- unaffected
firefox-esr17 18+ fixed

People

(Reporter: inferno, Assigned: roc)

Details

(5 keywords, Whiteboard: [asan][adv-main18+][adv-esr17+])

Attachments

(4 files)

Attached file 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
Severity: normal → critical
Component: General → Video/Audio
Keywords: crash, testcase
Product: Firefox → Core
Whiteboard: [asan]
The use-after-free stack has TrackUnionStream in it, so maybe it is related to bug 779715, which landed in 17.
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?
(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.
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
Attached file 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.
Attached file another crash
Attached patch fixSplinter Review
This fixes it for me in my asan build.
Assignee: nobody → roc
Attachment #677347 - Flags: review?(rjesup)
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?  :-)
Attachment #677347 - Flags: review?(rjesup) → review+
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 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.
Attachment #677347 - Flags: approval-mozilla-aurora?
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.
Attachment #677347 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/519848fd5eaa
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
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
I don't think this bug is in 17.
Oh, it is, never mind.
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
Attachment #677347 - Flags: approval-mozilla-esr17?
Attachment #677347 - Flags: approval-mozilla-esr17? → approval-mozilla-esr17+
Flags: sec-bounty?
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?
Keywords: regression
Flags: sec-bounty? → sec-bounty-
Keywords: sec-moderate
Whiteboard: [asan] → [asan][adv-main18+][adv-esr17+]
Alias: CVE-2013-0761
Group: core-security
Flags: sec-bounty-hof+
You need to log in before you can comment on or make changes to this bug.