1041466 - Heap-buffer-overflow in mozilla::MediaStreamGraphImpl::UpdateStreamOrder

Status: VERIFIED FIXED

(Core :: Web Audio, defect)

Description

[Atte Kettunen]

Attachment: Repro-file

Tested on:

OS: Ubuntu 12.04

Firefox: ASAN build from mozilla-central changeset: 195225:e743fd8c57ed


ASAN-trace:

==13096==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6060003fbca0 at pc 0x7f4daa440b9d bp 0x7f4d7c0b32b0 sp 0x7f4d7c0b32a8
WRITE of size 8 at 0x6060003fbca0 thread T32 (MediaStreamGrph)
    #0 0x7f4daa440b9c in mozilla::MediaStreamGraphImpl::UpdateStreamOrder() /home/attekett/firefox/content/media/MediaStreamGraph.cpp:669
    #1 0x7f4daa446073 in mozilla::MediaStreamGraphImpl::RunThread() /home/attekett/firefox/content/media/MediaStreamGraph.cpp:1376
    #2 0x7f4daa45b30c in mozilla::(anonymous namespace)::MediaStreamGraphInitThreadRunnable::Run() /home/attekett/firefox/content/media/MediaStreamGraph.cpp:1589
    #3 0x7f4da6891b96 in nsThread::ProcessNextEvent(bool, bool*) /home/attekett/firefox/xpcom/threads/nsThread.cpp:770
    #4 0x7f4da6773c78 in NS_ProcessNextEvent(nsIThread*, bool) /home/attekett/firefox/xpcom/glue/nsThreadUtils.cpp:265
    #5 0x7f4da70a827e in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /home/attekett/firefox/ipc/glue/MessagePump.cpp:326
    #6 0x7f4da70352de in MessageLoop::RunInternal() /home/attekett/firefox/ipc/chromium/src/base/message_loop.cc:229
    #7 0x7f4da688f0cd in nsThread::ThreadFunc(void*) /home/attekett/firefox/xpcom/threads/nsThread.cpp:347
    #8 0x7f4db23f0f25 in _pt_root /home/attekett/firefox/nsprpub/pr/src/pthreads/ptthread.c:212
    #9 0x7f4db5908e99 in start_thread ??:0
    #10 0x7f4db4a133fc in ?? ??:0

0x6060003fbca0 is located 0 bytes to the right of 64-byte region [0x6060003fbc60,0x6060003fbca0)
allocated by thread T32 (MediaStreamGrph) here:
    #0 0x45fb36 in __interceptor_realloc _asan_rtl_
    #1 0x7f4db5cfade4 in moz_xrealloc /home/attekett/firefox/memory/mozalloc/mozalloc.cpp:84
    #2 0x7f4da67684b9 in nsTArrayInfallibleAllocator::Realloc(void*, unsigned long) /home/attekett/firefox/objdir-ff-asan/content/media/../../dist/include/nsTArray.h:177
    #3 0x7f4daa469542 in mozilla::MediaStream** nsTArray_Impl<mozilla::MediaStream*, nsTArrayInfallibleAllocator>::AppendElements<mozilla::MediaStream*>(mozilla::MediaStream* const*, unsigned long) /home/attekett/firefox/objdir-ff-asan/content/media/../../dist/include/nsTArray.h:1234
    #4 0x7f4daa459197 in mozilla::MediaStream** nsTArray_Impl<mozilla::MediaStream*, nsTArrayInfallibleAllocator>::AppendElement<mozilla::MediaStream*>(mozilla::MediaStream* const&) /home/attekett/firefox/objdir-ff-asan/content/media/../../dist/include/nsTArray.h:1254
    #5 0x7f4daa445f89 in mozilla::MediaStreamGraphImpl::RunThread() /home/attekett/firefox/content/media/MediaStreamGraph.cpp:1370
    #6 0x7f4daa45b30c in mozilla::(anonymous namespace)::MediaStreamGraphInitThreadRunnable::Run() /home/attekett/firefox/content/media/MediaStreamGraph.cpp:1589
    #7 0x7f4da6891b96 in nsThread::ProcessNextEvent(bool, bool*) /home/attekett/firefox/xpcom/threads/nsThread.cpp:770
    #8 0x7f4da6773c78 in NS_ProcessNextEvent(nsIThread*, bool) /home/attekett/firefox/xpcom/glue/nsThreadUtils.cpp:265
    #9 0x7f4da70a827e in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /home/attekett/firefox/ipc/glue/MessagePump.cpp:326
    #10 0x7f4da70352de in MessageLoop::RunInternal() /home/attekett/firefox/ipc/chromium/src/base/message_loop.cc:229
    #11 0x7f4da688f0cd in nsThread::ThreadFunc(void*) /home/attekett/firefox/xpcom/threads/nsThread.cpp:347
    #12 0x7f4db23f0f25 in _pt_root /home/attekett/firefox/nsprpub/pr/src/pthreads/ptthread.c:212
    #13 0x7f4db5908e99 in start_thread ??:0

Thread T32 (MediaStreamGrph) created by T0 here:
    #0 0x423626 in __interceptor_pthread_create _asan_rtl_
    #1 0x7f4db23ed8ad in _PR_CreateThread /home/attekett/firefox/nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7f4db23ed42a in PR_CreateThread /home/attekett/firefox/nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7f4da6890072 in nsThread::Init() /home/attekett/firefox/xpcom/threads/nsThread.cpp:424
    #4 0x7f4da68945f4 in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) /home/attekett/firefox/xpcom/threads/nsThreadManager.cpp:269
    #5 0x7f4da6773409 in NS_NewThread(nsIThread**, nsIRunnable*, unsigned int) /home/attekett/firefox/xpcom/glue/nsThreadUtils.cpp:68
    #6 0x7f4daa44768a in tag_nsresult NS_NewNamedThread<16ul>(char const (&) [16ul], nsIThread**, nsIRunnable*, unsigned int) /home/attekett/firefox/objdir-ff-asan/content/media/../../dist/include/nsThreadUtils.h:75
    #7 0x7f4daa45af2e in mozilla::(anonymous namespace)::MediaStreamGraphStableStateRunnable::Run() /home/attekett/firefox/content/media/MediaStreamGraph.cpp:1657
    #8 0x7f4da9331705 in nsCOMPtr_base::assign_assuming_AddRef(nsISupports*) /home/attekett/firefox/objdir-ff-asan/widget/xpwidgets/../../dist/include/nsCOMPtr.h:458
    #9 0x7f4da9331cc5 in nsBaseAppShell::AfterProcessNextEvent(nsIThreadInternal*, unsigned int, bool) /home/attekett/firefox/widget/xpwidgets/nsBaseAppShell.h:93
    #10 0x7f4da6892082 in nsThread::ProcessNextEvent(bool, bool*) /home/attekett/firefox/xpcom/threads/nsThread.cpp:784
    #11 0x7f4da6773c78 in NS_ProcessNextEvent(nsIThread*, bool) /home/attekett/firefox/xpcom/glue/nsThreadUtils.cpp:265
    #12 0x7f4da70a73a9 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/attekett/firefox/ipc/glue/MessagePump.cpp:99
    #13 0x7f4da70352de in MessageLoop::RunInternal() /home/attekett/firefox/ipc/chromium/src/base/message_loop.cc:229
    #14 0x7f4da9330727 in nsBaseAppShell::Run() /home/attekett/firefox/widget/xpwidgets/nsBaseAppShell.cpp:164
    #15 0x7f4dabcb14e6 in nsAppStartup::Run() /home/attekett/firefox/toolkit/components/startup/nsAppStartup.cpp:278
    #16 0x7f4dabb2adf3 in XREMain::XRE_mainRun() /home/attekett/firefox/toolkit/xre/nsAppRunner.cpp:4013
    #17 0x7f4dabb2ba95 in XREMain::XRE_main(int, char**, nsXREAppData const*) /home/attekett/firefox/toolkit/xre/nsAppRunner.cpp:4084
    #18 0x7f4dabb2c3d7 in XRE_main /home/attekett/firefox/toolkit/xre/nsAppRunner.cpp:4298
    #19 0x479fa6 in do_main(int, char**, nsIFile*) /home/attekett/firefox/browser/app/nsBrowserApp.cpp:282
    #20 0x7f4db494076c in ?? ??:0

SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 ??
Shadow bytes around the buggy address:
  0x0c0c80077740: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c80077750: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c80077760: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c80077770: fa fa fa fa 00 00 00 00 00 00 00 fa fa fa fa fa
  0x0c0c80077780: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
=>0x0c0c80077790: 00 00 00 00[fa]fa fa fa 00 00 00 00 00 00 00 00
  0x0c0c800777a0: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c0c800777b0: 00 00 00 00 00 00 00 fa fa fa fa fa fd fd fd fd
  0x0c0c800777c0: fd fd fd fd fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c0c800777d0: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c0c800777e0: 00 00 00 00 00 00 00 00 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
  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
  Contiguous container OOB:fc
  ASan internal:           fe
==13096==ABORTING


Also reproduces with an assertion with Firefox ASAN debug build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan-debug/1405918761/

Assertion:

Assertion failure: orderedStreamCount == mFirstCycleBreaker, at /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/MediaStreamGraph.cpp:742

Comment 1

[Jesse Ruderman]

Might be a regression from bug 932400, which added the assertion.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: don't remove one too many streams from the SCC stack

Also reduce the number of ProcessedMediaStream* static_casts and use a new
variable name |removed| to emphasize what is happening.

Once the extra stream was removed, it wasn't ordered, and so another stream could end up occurring twice in mStreams and then bad things would happen.

Comment 3

[Karl Tomlinson (:karlt)]

Comment on attachment 8461427 [details] [diff] [review]
don't remove one too many streams from the SCC stack

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

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

The connection with the code change in the patch is not obvious, but with a bit of study of the surrounding code, someone could work out how to construct a UAF.
(The heap buffer overflow might be harder to exploit on its own because the bytes written are allocation addresses - i.e. not controlled by content.)

Which older supported branches are affected by this flaw?
33

If not all supported branches, which bug introduced the flaw?
bug 932400

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The same fix applies to 33 as to m-c.

How likely is this patch to cause regressions; how much testing does it need?
Unlikely.  The code is complex, but the change is small.

Comment 4

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8461427 [details] [diff] [review]
don't remove one too many streams from the SCC stack

sec-approval+ for trunk. Please nominate for Aurora as well so we can avoid shipping the issue.

Comment 5

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbfdfa2c7377

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cbfdfa2c7377

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8461427 [details] [diff] [review]
don't remove one too many streams from the SCC stack

Approval Request Comment
[Feature/regressing bug #]: bug 932400 
[User impact if declined]: sec-critical
[Describe test coverage new/current, TBPL]:
Testcase not committed for security reasons.
[Risks and why]: 
Low risk.  The code is complex, but the change is small.
[String/UUID change made/needed]: none.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e7e4f87bf560

Comment 11

[Ada [:adalucinet]]

On Ubuntu 13.10 64 bit with Nightly 2014-07-21 asan build, I get “Assertion failure: orderedStreamCount == mFirstCycleBreaker, at /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/content/media/MediaStreamGraph.cpp:742” when loading the Repro file.

Verified as fixed with Firefox 33 beta 8 (Build ID: 20140929180120), latest Nightly 2014-09-29 and Aurora 2014-09-29 on Ubuntu 12.04 64-bit and Ubuntu 13.10 64-bit.

Comment 12

[Karl Tomlinson (:karlt)]

The testcase in bug 1045650 reproduced more reliably, so I'll push that one.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40fe972a2dfb

Comment 14

[Karl Tomlinson (:karlt)]

Please ignore comment 12.  (I was thinking of something else.)
I've pushed testcases from both bugs.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40fe972a2dfb