Tested on: OS: Ubuntu 12.04 Firefox: ASAN build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan-debug/1387629746/ ASAN-report: ==772==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x63000028e280 at pc 0x7f5cfa080a41 bp 0x7f5cccfc8760 sp 0x7f5cccfc8758 READ of size 148 at 0x63000028e280 thread T35 (MediaStreamGrph) #0 0x7f5cfa080a40 in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromInputBuffer(mozilla::AudioChunk*, unsigned int, unsigned long, unsigned long, unsigned int) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:175:0 #1 0x7f5cfa080468 in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromBuffer(mozilla::AudioNodeStream*, mozilla::AudioChunk*, unsigned int, unsigned int*, long*, unsigned int, unsigned int) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:325:0 #2 0x7f5cfa07f904 in mozilla::dom::AudioBufferSourceNodeEngine::ProduceAudioBlock(mozilla::AudioNodeStream*, mozilla::AudioChunk const&, mozilla::AudioChunk*, bool*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:424:0 #3 0x7f5cf9fb5b7d in mozilla::AudioNodeStream::ProduceOutput(long, long) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/AudioNodeStream.cpp:434:0 #4 0x7f5cfa01c7e4 in mozilla::MediaStreamGraphImpl::ProduceDataForStreamsBlockByBlock(unsigned int, int, long, long) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/MediaStreamGraph.cpp:1088:0 #5 0x7f5cfa01d205 in mozilla::MediaStreamGraphImpl::RunThread() /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/MediaStreamGraph.cpp:1189:0 #6 0x7f5cfa040022 in mozilla::(anonymous namespace)::MediaStreamGraphInitThreadRunnable::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/MediaStreamGraph.cpp:1350:0 #7 0x7f5cf749ac33 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/xpcom/threads/nsThread.cpp:634:0 #8 0x7f5cf7380f06 in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/xpcom/glue/nsThreadUtils.cpp:263:0 #9 0x7f5cf7baad07 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/ipc/glue/MessagePump.cpp:301:0 #10 0x7f5cf7b18dd0 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-d-0000000000000/build/ipc/chromium/src/base/message_loop.cc:226:0 #11 0x7f5cf7b18bd4 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/ipc/chromium/src/base/message_loop.cc:193:0 #12 0x7f5cf749827a in nsThread::ThreadFunc(void*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/xpcom/threads/nsThread.cpp:258:0 #13 0x7f5d058a18b7 in _pt_root /builds/slave/m-cen-l64-asan-d-0000000000000/build/nsprpub/pr/src/pthreads/ptthread.c:205:0 #14 0x44cf33 in __asan::AsanThread::ThreadStart(unsigned long) _asan_rtl_:0 #15 0x7f5d08dc9e99 in start_thread ??:0 #16 0x7f5d07ed83fc in ?? ??:0 0x63000028e280 is located 57968 bytes to the right of 0-byte region [0x630000280010,0x630000280010) ==772==AddressSanitizer CHECK failed: /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:228 "((id)) != (0)" (0x0, 0x0) #0 0x44bd24 in __asan::AsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) _asan_rtl_:0 #1 0x450f51 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common.cc:60:0 #2 0x423252 in GetStackTraceFromId /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:228:0 #3 0x423252 in __asan::AsanChunkView::GetAllocStack(__sanitizer::StackTrace*) /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:238:0 #4 0x448e16 in __asan::DescribeHeapAddress(unsigned long, unsigned long) _asan_rtl_:0 #5 0x449f04 in __asan_report_error _asan_rtl_:0 #6 0x44b153 in __asan_report_load_n _asan_rtl_:0 #7 0x7f5cfa080a40 in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromInputBuffer(mozilla::AudioChunk*, unsigned int, unsigned long, unsigned long, unsigned int) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:175:0 #8 0x7f5cfa080468 in mozilla::dom::AudioBufferSourceNodeEngine::CopyFromBuffer(mozilla::AudioNodeStream*, mozilla::AudioChunk*, unsigned int, unsigned int*, long*, unsigned int, unsigned int) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:325:0 #9 0x7f5cfa07f904 in mozilla::dom::AudioBufferSourceNodeEngine::ProduceAudioBlock(mozilla::AudioNodeStream*, mozilla::AudioChunk const&, mozilla::AudioChunk*, bool*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/webaudio/AudioBufferSourceNode.cpp:424:0 #10 0x7f5cf9fb5b7d in mozilla::AudioNodeStream::ProduceOutput(long, long) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/AudioNodeStream.cpp:434:0 #11 0x7f5cfa01c7e4 in mozilla::MediaStreamGraphImpl::ProduceDataForStreamsBlockByBlock(unsigned int, int, long, long) /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/MediaStreamGraph.cpp:1088:0 #12 0x7f5cfa01d205 in mozilla::MediaStreamGraphImpl::RunThread() /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/MediaStreamGraph.cpp:1189:0 #13 0x7f5cfa040022 in mozilla::(anonymous namespace)::MediaStreamGraphInitThreadRunnable::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/content/media/MediaStreamGraph.cpp:1350:0 #14 0x7f5cf749ac33 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/xpcom/threads/nsThread.cpp:634:0 #15 0x7f5cf7380f06 in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/xpcom/glue/nsThreadUtils.cpp:263:0 #16 0x7f5cf7baad07 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/ipc/glue/MessagePump.cpp:301:0 #17 0x7f5cf7b18dd0 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-d-0000000000000/build/ipc/chromium/src/base/message_loop.cc:226:0 #18 0x7f5cf7b18bd4 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/ipc/chromium/src/base/message_loop.cc:193:0 #19 0x7f5cf749827a in nsThread::ThreadFunc(void*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/xpcom/threads/nsThread.cpp:258:0 #20 0x7f5d058a18b7 in _pt_root /builds/slave/m-cen-l64-asan-d-0000000000000/build/nsprpub/pr/src/pthreads/ptthread.c:205:0 #21 0x44cf33 in __asan::AsanThread::ThreadStart(unsigned long) _asan_rtl_:0 #22 0x7f5d08dc9e99 in start_thread ??:0 #23 0x7f5d07ed83fc in ?? ??:0

There has been a bug here, since the implementation in bug 864164, in that the garbage initial values of AudioBufferSourceNode::mOffset and mDuration are used instead of the values provided to Start() and already sent to the engine, since the implementation in bug 864164. However, that did not cause an overflow until changes from bug 937475. Setting tracking flags for the overflow. Content can control the size of the allocated buffer and read up to the same number of bytes again from the memory after the allocation.

Comment on attachment 8351119 [details] [diff] [review] always remember offset and duration from Start() [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 overflow is not immediately obvious from the patch, but not too difficult to work out either. Which older supported branches are affected by this flaw? 28 If not all supported branches, which bug introduced the flaw? bug 937475. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The same patch applies on 28. Not too risky for Aurora. How likely is this patch to cause regressions; how much testing does it need? Unlikely to cause regressions in expected use cases. The patch doesn't change the behaviour in expected uses cases, only in the corner case presented in this bug.

Comment on attachment 8351119 [details] [diff] [review] always remember offset and duration from Start() sec-approval+ for trunk. Please make an Aurora patch and nominate it. Once things are in trunk and clear, it can be approved to go in.

Comment on attachment 8351119 [details] [diff] [review] always remember offset and duration from Start() [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 937475. User impact if declined: security risk. Testing completed (on m-c, etc.): on m-c. Risk to taking this patch (and alternatives if risky): Unlikely to cause regressions in expected use cases. The patch doesn't change the behaviour in expected uses cases, only in the corner case presented in this bug. String or IDL/UUID changes made by this patch: none.

Confirmed crash in ASan FF29, 2013-12-19. Verified fixed in ASan FF28 and FF29, 2014-01-18.

I pushed the test because this has been fixed on Aurora for a month, and I want to be sure that other changes I make to AudioBufferSourceNode don't regress this.

tests landed as https://hg.mozilla.org/mozilla-central/rev/3eb7ea1fb2d7