Closed Bug 1448705 Opened 6 years ago Closed 6 years ago

speex: heap-buffer-overflow in resampler_basic_direct_single

Categories

(Core :: Audio/Video: Playback, defect, P1)

Product:
Core
Component:
Audio/Video: Playback
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox-esr52 60+ verified
firefox59 --- wontfix
firefox60 + verified
firefox61 + verified

People

(Reporter: tsmith, Assigned: jesup)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [adv-main60+][adv-esr52.8+])

Crash Data

Attachments

(4 files)

testresample.c
3.83 KB, text/x-csrc
Details
testcase.html
183 bytes, text/html
Details
testcase.wav
46.10 KB, audio/x-wav
Details
use input latency for draining
1.25 KB, patch
jya
: review+
abillings
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr52+
abillings
: sec-approval+
Details | Diff | Splinter Review 
Found in m-c: 20180325-0707db8f028d
I don't have a a testcase at the moment.

==3980==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6320002ce2bc at pc 0x7fc3857aa226 bp 0x7fc3395cdb90 sp 0x7fc3395cdb88
WRITE of size 4 at 0x6320002ce2bc thread T2042 (MediaPl~back #2)
    #0 0x7fc3857aa225 in resampler_basic_direct_single src/media/libspeex_resampler/src/resample.c:371:38
    #1 0x7fc3857a3a33 in speex_resampler_process_native src/media/libspeex_resampler/src/resample.c:903:17
    #2 0x7fc3857a3a33 in moz_speex_resampler_process_float src/media/libspeex_resampler/src/resample.c:966
    #3 0x7fc3857a857f in moz_speex_resampler_process_interleaved_float src/media/libspeex_resampler/src/resample.c
    #4 0x7fc381e9ba29 in mozilla::AudioConverter::ResampleAudio(void*, void const*, unsigned long) src/dom/media/AudioConverter.cpp:308:7
    #5 0x7fc381e9bfaa in mozilla::AudioConverter::DrainResampler(void*) src/dom/media/AudioConverter.cpp:363:12
    #6 0x7fc382406892 in mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float> mozilla::AudioConverter::Process<(mozilla::AudioConfig::SampleFormat)6, float>(mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float> const&) src/obj-firefox/dist/include/AudioConverter.h:170:16
    #7 0x7fc3823eb03a in mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float> mozilla::AudioConverter::Process<(mozilla::AudioConfig::SampleFormat)6, float>(mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float>&&) src/obj-firefox/dist/include/AudioConverter.h:138:12
    #8 0x7fc3823ea126 in mozilla::media::AudioSink::DrainConverter(unsigned int) src/dom/media/mediasink/AudioSink.cpp:527:17
    #9 0x7fc3823e741c in mozilla::media::AudioSink::NotifyAudioNeeded() src/dom/media/mediasink/AudioSink.cpp:471:5
    #10 0x7fc3823e2edb in mozilla::media::AudioSink::Init(mozilla::media::MediaSink::PlaybackParams const&) src/dom/media/mediasink/AudioSink.cpp:93:3
    #11 0x7fc3823ed230 in mozilla::media::AudioSinkWrapper::Start(mozilla::media::TimeUnit const&, mozilla::MediaInfo const&) src/dom/media/mediasink/AudioSinkWrapper.cpp:196:31
    #12 0x7fc3823fb71f in mozilla::media::VideoSink::Start(mozilla::media::TimeUnit const&, mozilla::MediaInfo const&) src/dom/media/mediasink/VideoSink.cpp:172:15
    #13 0x7fc381fb546d in mozilla::MediaDecoderStateMachine::StartMediaSink() src/dom/media/MediaDecoderStateMachine.cpp:3328:17
    #14 0x7fc381fa25c5 in mozilla::MediaDecoderStateMachine::MaybeStartPlayback() src/dom/media/MediaDecoderStateMachine.cpp:2958:3
    #15 0x7fc38206420f in mozilla::MediaDecoderStateMachine::CompletedState::Step() src/dom/media/MediaDecoderStateMachine.cpp:1924:16
    #16 0x7fc381fa07e1 in Enter src/dom/media/MediaDecoderStateMachine.cpp:1903:5
    #17 0x7fc381fa07e1 in CallEnterMemberFunction<mozilla::MediaDecoderStateMachine::CompletedState> src/dom/media/MediaDecoderStateMachine.cpp:270
    #18 0x7fc381fa07e1 in decltype (ReturnTypeHelper(&mozilla::MediaDecoderStateMachine::CompletedState::Enter)) mozilla::MediaDecoderStateMachine::StateObject::SetState<mozilla::MediaDecoderStateMachine::CompletedState>() src/dom/media/MediaDecoderStateMachine.cpp:308
    #19 0x7fc381fa3cf5 in mozilla::MediaDecoderStateMachine::DecodingState::HandleEndOfAudio() src/dom/media/MediaDecoderStateMachine.cpp:2385:5
    #20 0x7fc38207e977 in mozilla::MediaDecoderStateMachine::RequestAudioData()::$_15::operator()(mozilla::MediaResult const&) const src/dom/media/MediaDecoderStateMachine.cpp
    #21 0x7fc38207d603 in InvokeMethod<(lambda at src/dom/media/MediaDecoderStateMachine.cpp:3224:5), void ((lambda at src/dom/media/MediaDecoderStateMachine.cpp:3224:5)::*)(const mozilla::MediaResult &) const, mozilla::MediaResult> src/obj-firefox/dist/include/mozilla/MozPromise.h:521:12
    #22 0x7fc38207d603 in InvokeCallbackMethod<false, (lambda at src/dom/media/MediaDecoderStateMachine.cpp:3224:5), void ((lambda at src/dom/media/MediaDecoderStateMachine.cpp:3224:5)::*)(const mozilla::MediaResult &) const, mozilla::MediaResult, RefPtr<mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::Private> > src/obj-firefox/dist/include/mozilla/MozPromise.h:566
    #23 0x7fc38207d603 in mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ThenValue<mozilla::MediaDecoderStateMachine::RequestAudioData()::$_14, mozilla::MediaDecoderStateMachine::RequestAudioData()::$_15>::DoResolveOrRejectInternal(mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ResolveOrRejectValue&) src/obj-firefox/dist/include/mozilla/MozPromise.h:772
    #24 0x7fc38208023e in mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ThenValueBase::ResolveOrRejectRunnable::Run() src/obj-firefox/dist/include/mozilla/MozPromise.h:403:21
    #25 0x7fc37bac4868 in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:214:37
    #26 0x7fc37bad17ba in mozilla::TaskQueue::Runner::Run() src/xpcom/threads/TaskQueue.cpp:243:12
    #27 0x7fc37bb0202c in nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp:228:14
    #28 0x7fc37bb0272c in non-virtual thunk to nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp
    #29 0x7fc37baeafb8 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1096:14
    #30 0x7fc37bb07320 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
    #31 0x7fc37c9ca39b in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:334:20
    #32 0x7fc37c9192f9 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
    #33 0x7fc37c9192f9 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
    #34 0x7fc37c9192f9 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
    #35 0x7fc37bae5408 in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:425:11
    #36 0x7fc39955047e in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #37 0x7fc39cb4a6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #38 0x7fc39bbd341c in clone /build/glibc-Cl5G7W/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

0x6320002ce2be is located 0 bytes to the right of 88766-byte region [0x6320002b8800,0x6320002ce2be)
allocated by thread T2042 (MediaPl~back #2) here:
    #0 0x4c7303 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3
    #1 0x7fc38204c1b9 in operator new[] src/obj-firefox/dist/include/mozilla/mozalloc.h:174:12
    #2 0x7fc38204c1b9 in MakeUniqueFallible<unsigned char []> src/obj-firefox/dist/include/mozilla/UniquePtrExtensions.h:33
    #3 0x7fc38204c1b9 in mozilla::AlignedBuffer<float, 32>::EnsureCapacity(unsigned long) src/dom/media/MediaData.h:248
    #4 0x7fc3824065dd in SetLength src/obj-firefox/dist/include/MediaData.h:134:31
    #5 0x7fc3824065dd in mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float> mozilla::AudioConverter::Process<(mozilla::AudioConfig::SampleFormat)6, float>(mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float> const&) src/obj-firefox/dist/include/AudioConverter.h:164
    #6 0x7fc3823eb03a in mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float> mozilla::AudioConverter::Process<(mozilla::AudioConfig::SampleFormat)6, float>(mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float>&&) src/obj-firefox/dist/include/AudioConverter.h:138:12
    #7 0x7fc3823ea126 in mozilla::media::AudioSink::DrainConverter(unsigned int) src/dom/media/mediasink/AudioSink.cpp:527:17
    #8 0x7fc3823e741c in mozilla::media::AudioSink::NotifyAudioNeeded() src/dom/media/mediasink/AudioSink.cpp:471:5
    #9 0x7fc3823e2edb in mozilla::media::AudioSink::Init(mozilla::media::MediaSink::PlaybackParams const&) src/dom/media/mediasink/AudioSink.cpp:93:3
    #10 0x7fc3823ed230 in mozilla::media::AudioSinkWrapper::Start(mozilla::media::TimeUnit const&, mozilla::MediaInfo const&) src/dom/media/mediasink/AudioSinkWrapper.cpp:196:31
    #11 0x7fc3823fb71f in mozilla::media::VideoSink::Start(mozilla::media::TimeUnit const&, mozilla::MediaInfo const&) src/dom/media/mediasink/VideoSink.cpp:172:15
    #12 0x7fc381fb546d in mozilla::MediaDecoderStateMachine::StartMediaSink() src/dom/media/MediaDecoderStateMachine.cpp:3328:17
    #13 0x7fc381fa25c5 in mozilla::MediaDecoderStateMachine::MaybeStartPlayback() src/dom/media/MediaDecoderStateMachine.cpp:2958:3
    #14 0x7fc38206420f in mozilla::MediaDecoderStateMachine::CompletedState::Step() src/dom/media/MediaDecoderStateMachine.cpp:1924:16
    #15 0x7fc381fa07e1 in Enter src/dom/media/MediaDecoderStateMachine.cpp:1903:5
    #16 0x7fc381fa07e1 in CallEnterMemberFunction<mozilla::MediaDecoderStateMachine::CompletedState> src/dom/media/MediaDecoderStateMachine.cpp:270
    #17 0x7fc381fa07e1 in decltype (ReturnTypeHelper(&mozilla::MediaDecoderStateMachine::CompletedState::Enter)) mozilla::MediaDecoderStateMachine::StateObject::SetState<mozilla::MediaDecoderStateMachine::CompletedState>() src/dom/media/MediaDecoderStateMachine.cpp:308
    #18 0x7fc381fa3cf5 in mozilla::MediaDecoderStateMachine::DecodingState::HandleEndOfAudio() src/dom/media/MediaDecoderStateMachine.cpp:2385:5
    #19 0x7fc38207e977 in mozilla::MediaDecoderStateMachine::RequestAudioData()::$_15::operator()(mozilla::MediaResult const&) const src/dom/media/MediaDecoderStateMachine.cpp
    #20 0x7fc38207d603 in InvokeMethod<(lambda at src/dom/media/MediaDecoderStateMachine.cpp:3224:5), void ((lambda at src/dom/media/MediaDecoderStateMachine.cpp:3224:5)::*)(const mozilla::MediaResult &) const, mozilla::MediaResult> src/obj-firefox/dist/include/mozilla/MozPromise.h:521:12
    #21 0x7fc38207d603 in InvokeCallbackMethod<false, (lambda at src/dom/media/MediaDecoderStateMachine.cpp:3224:5), void ((lambda at src/dom/media/MediaDecoderStateMachine.cpp:3224:5)::*)(const mozilla::MediaResult &) const, mozilla::MediaResult, RefPtr<mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::Private> > src/obj-firefox/dist/include/mozilla/MozPromise.h:566
    #22 0x7fc38207d603 in mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ThenValue<mozilla::MediaDecoderStateMachine::RequestAudioData()::$_14, mozilla::MediaDecoderStateMachine::RequestAudioData()::$_15>::DoResolveOrRejectInternal(mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ResolveOrRejectValue&) src/obj-firefox/dist/include/mozilla/MozPromise.h:772
    #23 0x7fc38208023e in mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ThenValueBase::ResolveOrRejectRunnable::Run() src/obj-firefox/dist/include/mozilla/MozPromise.h:403:21
    #24 0x7fc37bac4868 in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:214:37
    #25 0x7fc37bad17ba in mozilla::TaskQueue::Runner::Run() src/xpcom/threads/TaskQueue.cpp:243:12
    #26 0x7fc37bb0202c in nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp:228:14
    #27 0x7fc37bb0272c in non-virtual thunk to nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp
    #28 0x7fc37baeafb8 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1096:14
    #29 0x7fc37bb07320 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
    #30 0x7fc37c9ca39b in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:334:20
    #31 0x7fc37c9192f9 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
    #32 0x7fc37c9192f9 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
    #33 0x7fc37c9192f9 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
    #34 0x7fc37bae5408 in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:425:11
    #35 0x7fc39955047e in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #36 0x7fc39cb4a6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Thread T2042 (MediaPl~back #2) created by T0 here:
    #0 0x4b065d in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:204:3
    #1 0x7fc39954d1cf in _PR_CreateThread src/nsprpub/pr/src/pthreads/ptthread.c:433:14
    #2 0x7fc39954cdbe in PR_CreateThread src/nsprpub/pr/src/pthreads/ptthread.c:518:12
    #3 0x7fc37bae7383 in nsThread::Init(nsTSubstring<char> const&) src/xpcom/threads/nsThread.cpp:612:8
    #4 0x7fc37baf0f6a in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) src/xpcom/threads/nsThreadManager.cpp:471:22
    #5 0x7fc37bb00d8f in NS_NewNamedThread src/xpcom/threads/nsThreadUtils.cpp:143:45
    #6 0x7fc37bb00d8f in nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>, unsigned int) src/xpcom/threads/nsThreadPool.cpp:109
    #7 0x7fc37bb028e6 in nsThreadPool::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) src/xpcom/threads/nsThreadPool.cpp:277:5
    #8 0x7fc37bad01fb in mozilla::TaskQueue::DispatchLocked(nsCOMPtr<nsIRunnable>&, mozilla::AbstractThread::DispatchReason) src/xpcom/threads/TaskQueue.cpp:125:26
    #9 0x7fc37baf3711 in mozilla::TaskQueue::Dispatch(already_AddRefed<nsIRunnable>, mozilla::AbstractThread::DispatchReason) src/obj-firefox/dist/include/mozilla/TaskQueue.h:71:14
    #10 0x7fc37bac2b6e in DispatchTaskGroup src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:265:20
    #11 0x7fc37bac2b6e in mozilla::AutoTaskDispatcher::~AutoTaskDispatcher() src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:90
    #12 0x7fc37bac21ec in reset src/obj-firefox/dist/include/mozilla/Maybe.h:488:17
    #13 0x7fc37bac21ec in mozilla::EventTargetWrapper::FireTailDispatcher() src/xpcom/threads/AbstractThread.cpp:75
    #14 0x7fc37bac6434 in applyImpl<mozilla::EventTargetWrapper, void (mozilla::EventTargetWrapper::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1164:12
    #15 0x7fc37bac6434 in apply<mozilla::EventTargetWrapper, void (mozilla::EventTargetWrapper::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1170
    #16 0x7fc37bac6434 in mozilla::detail::RunnableMethodImpl<mozilla::EventTargetWrapper*, void (mozilla::EventTargetWrapper::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1215
    #17 0x7fc37b940cd0 in mozilla::CycleCollectedJSContext::ProcessStableStateQueue() src/xpcom/base/CycleCollectedJSContext.cpp:312:12
    #18 0x7fc37b942e65 in mozilla::CycleCollectedJSContext::AfterProcessTask(unsigned int) src/xpcom/base/CycleCollectedJSContext.cpp:377:3
    #19 0x7fc37d376c1d in XPCJSContext::AfterProcessTask(unsigned int) src/js/xpconnect/src/XPCJSContext.cpp:1252:30
    #20 0x7fc37baeba83 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1133:24
    #21 0x7fc37bb07320 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
    #22 0x7fc37c9c93aa in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #23 0x7fc37c9192f9 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
    #24 0x7fc37c9192f9 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
    #25 0x7fc37c9192f9 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
    #26 0x7fc38364fa6a in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27
    #27 0x7fc3876e7cdb in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:290:30
    #28 0x7fc3878f406c in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4766:22
    #29 0x7fc3878f71a6 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4911:8
    #30 0x7fc3878f8664 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5003:21
    #31 0x4f6d45 in do_main src/browser/app/nsBrowserApp.cpp:231:22
    #32 0x4f6d45 in main src/browser/app/nsBrowserApp.cpp:304
    #33 0x7fc39baec82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291

SUMMARY: AddressSanitizer: heap-buffer-overflow src/media/libspeex_resampler/src/resample.c:371:38 in resampler_basic_direct_single
Shadow bytes around the buggy address:
  0x0c6480051c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c6480051c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c6480051c20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c6480051c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c6480051c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c6480051c50: 00 00 00 00 00 00 00[06]fa fa fa fa fa fa fa fa
  0x0c6480051c60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c6480051c70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c6480051c80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c6480051c90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c6480051ca0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==3980==ABORTING
Looks like this has been happening for a while.
Crash Signature: [@ resampler_basic_direct_single]
status-firefox59: --- → affected
status-firefox60: --- → affected
Any information about what context the resampler code is being used. While it hasn't had much maintenance in the last years, I've also had very few bug reports. It's possible that Firefox is really exercising a corner case and triggering a bug (we've had some of those with strange user-defined rates), but if it's just being used in a straightforward way like constant-rate 48 kHz to 44.1 kHz conversion, then it's much more likely that the problem is in the way it's being used. For example, passing a corrupted state as the "st" parameter, or the wrong array as "in" or "out" (or with the wrong size), can easily explain a crash.
Note: the frequencies aren't directly visible, they're in allocated memory.

For https://crash-stats.mozilla.com/report/index/3de3665c-73d1-4e37-a134-7547a0180324#tab-details

in cubeb_resampler_speex_one_way<float>:
   in_len = 971
   out_len = 176
   istride_save = ostride_save = 1
   i = 0

in moz_speex_resampler_process_float():
  istride = 2
  ilen = 491
  ochunk = 88
  olen = 88
  in_len = 971
  ichunk = 160
  channel_index = 0
  xlen = 160

speex_resampler_process_native()
  N = 488
  in_len = 160
  out_len = 88
  out = x211079fef40

in resampler_basic_direct_single():
  den_rate = 80
  frac_advance = 41
  in_len = 160
  last_sample = 142
  N = 488
  out = (same as above)
  out_len = 88
  out_sample = 25
  out_stride = 2
  samp_frac_num = 73
  
Actually crashes on the out[out_stride * out_sample++] = sum, I believe.
Flags: needinfo?(jmvalin)
From looking at the values above, everything looks normal. If the crash is indeed happening in out[out_stride * out_sample++], then I can only see two options:
1) The output_buffer array being passed as the "out" argument is bad
2) The output_buffer is smaller than what's being told to the resampler.
Regarding 2), make sure that the size being passed is really *per channel*. If there are N channels, then the buffer needs to have  size N*(*out_len), not just (*out_len).

From what I can see, this is a pretty straightforward use of the library, so if it was a bug in the library it would have crashed for everyone else. That's why I suspect it's a problem with how it's being called.
Flags: needinfo?(jmvalin)
Randell, could we add a MOZ_ASSERT or two in the mozilla patch to hopefully catch this API misuse?
Did you confirm it's indeed in the bug in the code calling the resampler?
No, that's what I'm hoping the assertions would help with. I have a basic API fuzzer but I hasn't found anything. I'll post it in a few.
By API fuzzer, you mean a fuzzer for the resampler itself? If so this is something that would be very useful to merge upstream.
Attached file testresample.c — 
Here is what I did.

I reused testresample.c and it is very targeted atm. This basically uses the first VECTOR_LENGTH (in this case 6) of unsigned ints as a vector (data to use to make random decisions with) and everything following as data. Running this will AFL quickly finds some code paths but no bugs. I let a earlier version run for 2 days.
A READ failure in 60b6 looks different: https://crash-stats.mozilla.com/report/index/aece1f26-3581-414b-9204-fd1e30180324#tab-details
It's crashing on mov ebx,dword ptr [rax+rdi*4].  rax was loaded from rt; it appears to be st->samp_frac_num (x48 offset, could be last_sample if the opt swapped the order); both are indexed by channel_index, which is 0 (in edi).  rax is 0x000401e633024680, which appears reasonable if you ignore a 1-bit-error.  So ignore that one.  We need to buy everyone ECC ram...
Looking at the 3-month view, I see all the write failures *stopped* in 60 (and before 60, 95% of the failures were WRITE).  All failures in 60 (3 of them) are READ (one the one right above, bit-flip).  There may have been a fix in the calling code... Paul, Alex?  Rate is low, so it could just be luck; there were few crashes in 59b as well - but they were all WRITE failures.  There were MANY crashes in 58 and 58b; all or almost all were WRITE.  This explains the recent drop in crash rate when 59 shipped.
Flags: needinfo?(padenot)
Flags: needinfo?(achronop)
Keywords: sec-high
This is Jean-Yves's code, best to have him cc-ed here. However, having a quick look, it seems fishy to allocate an output buffer using `speex_resampler_get_input_latency` here [0]. It's probably best to use the `_output_` version of this.

Apparently, we're also starting and stopping this `AudioSink` synchronously on the same call, this can maybe help to have a way to repro.

[0]: https://searchfox.org/mozilla-central/source/dom/media/AudioConverter.cpp#357
Flags: needinfo?(padenot) → needinfo?(jyavenard)
For comment 10, something is weird: the stack shows we're in a path were we dealing with an input-only stream (we don't use those in Gecko, it's either output only or duplex), _after_ a check that we're not duplex, and we're jumping into the callback set for a stream that is output only.

We're always making those decisions based on data that is immutable during the lifetime of a stream, so all this makes very little sense.
Well, the size of the buffer doesn't really matter, as long as the value passed with out_len is correct. The buffer needs to be at least (*out_len)*channels*sizeof(*out) bytes.
(In reply to Paul Adenot (:padenot) from comment #13)
> For comment 10, something is weird: the stack shows we're in a path were we
> dealing with an input-only stream (we don't use those in Gecko, it's either
> output only or duplex), _after_ a check that we're not duplex, and we're
> jumping into the callback set for a stream that is output only.

Comment 10's crash can probably be ignored - as I said, there's a 1-bit-error in the address it faulted on.  And I think we can end up with input-only streams, though maybe that was changed.  but that may be irrelevant.  And I didn't vet if the stack shown in crash-stats matches the stack in VS in the dump; once I was sure it was a bit-flip error I stopped.

Like I said, the interesting part is that WRITE crashes stopped after 59 it appears.
Attached file testcase.html — 

    
    

    
  

      
      
      
      

        Attached audio
          
        testcase.wav
        — 
      

    


  

    
  
Flags: in-testsuite?
Keywords: testcase-wanted → testcase

    
    

    
  
In inbound from 3/15, I get DECODING_METADATA Decode metadata failed, shutting down decoder: file /home/jesup/src/mozilla/asan/dom/media/MediaDecoderStateMachine.cpp, line 379

    
    

    
  
In m-c from a day or two ago, I get:
[29863, MediaPlayback #4] WARNING: 7fffb31b9e80 OpenCubeb() failed to init cubeb: file /home/jesup/src/mozilla/head/dom/media/AudioStream.cpp, line 390
[29863, MediaPlayback #3] WARNING: Decoder=7fffb4411200 [OnMediaSinkAudioError]: file /home/jesup/src/mozilla/head/dom/media/MediaDecoderStateMachine.cpp, line 3746
Flags: needinfo?(twsmith)

    
    

    
  
Was it an ASan build you used? I just doubled checked and I can repro it consistently on the latest ASan m-c build from TC[1].

BuildID=20180331213254
SourceStamp=3f9a70b125f6fb9be2b209159657fd7ae5515c01

[1] https://tools.taskcluster.net/index/gecko.v2.mozilla-central.latest.firefox/linux64-asan-opt
Flags: needinfo?(twsmith)

    
    

    
  
I tried first on an ASAN build, then on a non-ASAN opt m-c build.  I'll rebuild asan with an fresh pull

    
    

    
  
A fresh asan pull hits this bug.  I'm not sure how old the last pull was (certainly a few weeks, perhaps a month).  Rebuilding ASAN with debug to track it down

    
    

    
  


  
Note that DrainResampler() uses speex_resampler_get_input_latency() to create a 'fake' (0's) input buffer, then feeds that into ResampleAudio() with the output buffer, so we need to make sure that the output buffer is sized based on that input size, not the resampler output latency (which is almost but not quite the same)

        Attachment #8964398 -
        Flags: review?(jyavenard)

        Attachment #8964398 -
        Flags: review?(jmvalin)
Assignee: nobody → rjesup
Status: NEW → ASSIGNED

    
    

    
  
ok, this is fallout from Bug 1264199 which landed a 2 years ago in FF 49.

The problem is that when we're going to drain (frames = 0), we allocate space using speex_resampler_get_output_latency() (176 in this case).  We then call Drain, which uses speex_resampler_get_input_latency() (32) to pass into ResampleAudio(), which causes it to calculate the output buffer per channel using (uint64_t)aFrames * mOut.Rate() / mIn.Rate() + 1; which is 177.  (this difference is in AudioConverter::ResampleRecipientFrames() which "knows" that 0 means Drain will be called, and calculates the buffer accordingly.

We can fix this in a couple of ways, but it amounts to using speex_resampler_get_input_latency() for the calculation (we could use max(speex_resampler_get_output_latency(), speex_resampler_get_input_latency()*mOut.Rate()/mIn.Rate()), but that seems unnecessary).
Flags: needinfo?(jyavenard)
Flags: needinfo?(achronop)

    
    

    
  
(In reply to Paul Adenot (:padenot) from comment #12)
> This is Jean-Yves's code, best to have him cc-ed here. However, having a
> quick look, it seems fishy to allocate an output buffer using
> `speex_resampler_get_input_latency` here [0]. It's probably best to use the
> `_output_` version of this.
> 

We want to drain the resampler, and to do so we have to feed it 0s.
This code check what the input latency is, in order to calculate a buffer made of zeros.
We then feed the resampler that content.

So it doesn't really matter if you call speex_resampler_get_input_latency or speex_resampler_get_output_latency, frames here is the size of the input buffer, which will be used for reading only. Nothing will be written to that allocated buffer.
buffer is *not* an output buffer, it's an input buffer. 

Not looking at the stack trace, but the comments here: buffer should never be written to, and if speex reads more than it was told to.

If it's the size of the output buffer that isn't big enough, then this should be fixed there:
https://searchfox.org/mozilla-central/source/dom/media/AudioConverter.h#161

        Attachment #8964398 -
        Flags: review?(jyavenard) → review+

    
    

    
  
> So it doesn't really matter if you call speex_resampler_get_input_latency or
> speex_resampler_get_output_latency, frames here is the size of the input
> buffer, which will be used for reading only. Nothing will be written to that
> allocated buffer.
> buffer is *not* an output buffer, it's an input buffer. 
> 
> Not looking at the stack trace, but the comments here: buffer should never
> be written to, and if speex reads more than it was told to.

Right, and it doesn't.  That's fine, and get_input_latency() is a correct amount of data to feed in to ensure all real data has been output from the resampler.

> If it's the size of the output buffer that isn't big enough, then this
> should be fixed there:
> https://searchfox.org/mozilla-central/source/dom/media/AudioConverter.h#161

That is effectively what this fix does: we only modify the frames==0 part of ResampleRecipientFrames() to use the same input_latency we'll later use in DrainResampler to create the input frames for draining (since the two should match).

    
    

    
  
Comment on attachment 8964398 [details] [diff] [review]
use input latency for draining

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

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?  The comments make it clear this is affecting the size of output buffers.  That could be a clue that someone should poke at this to find a buffer-overrun.  Removing the comment would make it fairly obtuse; I'm good with removing it.  the checkin comment seems ok.

Which older supported branches are affected by this flaw? all

If not all supported branches, which bug introduced the flaw? bug 1264199 (FF 49)

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?  Should be trivial; this code hasn't changed.

How likely is this patch to cause regressions; how much testing does it need?  Regressions are unlikely. Verification that the testcase is fixed would be good (locally verified).  also, earlier testing implied that this exact testcase may not fail on FF60 or very early 61's; something seemed to be blocking decoding the file at all.  However, this bug appears to go back to FF49, and in the field we see crashes going back a long ways.  There are a few crashes in 38/42/45/47 in crash-stats; it's possible older code had a similar bug, or these may be bit-flip or other errors not related to this bug.  One of the 3 crashes in 60 is a bit-flip; this code is invoked quite a bit.

        Attachment #8964398 -
        Flags: sec-approval?

        Attachment #8964398 -
        Flags: approval-mozilla-esr52?

        Attachment #8964398 -
        Flags: approval-mozilla-beta?

    
    

    
  
As for why it's less common now: we're only using AudioConverter for a subset of cases where audio content changes since cubeb was opened; for all other cases it's just fed to cubeb.  Note cubeb has resampling support, but has no concept of "drain" (which perhaps is a minor bug) in some cases), so it wouldn't have this exact problem.

    
    

    
  
sec-approval+ for trunk. I'll approve the beta checkin too. Release management needs to do ESR52.

          status-firefox59:
          affected → wontfix

          status-firefox-esr52:
          --- → affected

          tracking-firefox60:
          --- → +

          tracking-firefox61:
          --- → +

          tracking-firefox-esr52:
          --- → 60+

        Attachment #8964398 -
        Flags: sec-approval?

        Attachment #8964398 -
        Flags: sec-approval+

        Attachment #8964398 -
        Flags: approval-mozilla-beta?

        Attachment #8964398 -
        Flags: approval-mozilla-beta+

    
  

        Attachment #8964398 -
        Flags: review?(jmvalin)
Priority: -- → P1

    
    

    
  
Comment on attachment 8964398 [details] [diff] [review]
use input latency for draining

Approved for ESR 52.8 as well once it lands on trunk.

        Attachment #8964398 -
        Flags: approval-mozilla-esr52? → approval-mozilla-esr52+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/b2904f128f85
Status: ASSIGNED → RESOLVED
Closed: 6 years ago

          status-firefox61:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Group: media-core-security → core-security-release
Whiteboard: [adv-main60+][adv-esr52.8+]

    
    

    
  
I have managed to reproduce the issue described in comment 0 using Firefox 61.0a1 (BuildId:20180325074135) Asan build.

This issue is verified fixed on Firefox 61.0a1(BuildId:20180430095344) and Firefox 60.0b16(BuildId:20180428164338) using Ubuntu 16.04 64bit.

Leaving a ni on myself to verify this on 52.8.0.

          status-firefox60:
          fixed → verified

          status-firefox61:
          fixed → verified
Flags: needinfo?(emil.ghitta)

    
    

    
  
This issue is verified fixed using Firefox 52.8.0 esr Asan build on Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED

          status-firefox-esr52:
          fixed → verified
Flags: needinfo?(emil.ghitta)
Group: core-security-release

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: