Closed Bug 1266260 Opened 4 years ago Closed 4 years ago

Speex: heap-buffer-overflow crash [@moz_speex_resampler_process_float]

Categories

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

x86_64
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox46 --- unaffected
firefox47 --- unaffected
firefox48 + verified
firefox49 + verified
firefox-esr45 --- unaffected

People

(Reporter: posidron, Assigned: jya)

References

(Depends on 1 open bug)

Details

(4 keywords, Whiteboard: take bug 1274083 also when landing on Fx48)

Attachments

(8 files, 2 obsolete files)

The following testcase crashes on en-us.linux-x86_64-asan.tar.bz2 revision d6655d9a086b8850cfe8fb16f11af87f879e59be

See attachment.

Backtrace:

==15325==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61a000108c7c at pc 0x7fa6f431e030 bp 0x7fa6aaa1f310 sp 0x7fa6aaa1f308
WRITE of size 4 at 0x61a000108c7c thread T4627 (MediaPl~back #1)
    #0 0x7fa6f431e02f in moz_speex_resampler_process_float /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libspeex_resampler/src/resample.c:956
    #1 0x7fa6f431fcec in moz_speex_resampler_process_interleaved_float /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libspeex_resampler/src/resample.c:1064
    #2 0x7fa6f0a76dc4 in mozilla::AudioConverter::ResampleAudio(void*, void const*, unsigned long) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/AudioConverter.cpp:258
    #3 0x7fa6f0db66f7 in mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float> mozilla::AudioConverter::Process<(mozilla::AudioConfig::SampleFormat)6, float>(mozilla::AudioDataBuffer<(mozilla::AudioConfig::SampleFormat)6, float>&&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/AudioConverter.h:137
    #4 0x7fa6f0db2aa4 in mozilla::media::DecodedAudioDataSink::NotifyAudioNeeded() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/mediasink/DecodedAudioDataSink.cpp:415
    #5 0x7fa6f0db098b in mozilla::media::DecodedAudioDataSink::Init(mozilla::media::MediaSink::PlaybackParams const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/mediasink/DecodedAudioDataSink.cpp:88
    #6 0x7fa6f0daeb4f in mozilla::media::AudioSinkWrapper::Start(long, mozilla::MediaInfo const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/mediasink/AudioSinkWrapper.cpp:191
    #7 0x7fa6f0dc1627 in mozilla::media::VideoSink::Start(long, mozilla::MediaInfo const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/mediasink/VideoSink.cpp:162
    #8 0x7fa6f0b4527a in mozilla::MediaDecoderStateMachine::StartMediaSink() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp:1703
    #9 0x7fa6f0b44e8a in mozilla::MediaDecoderStateMachine::MaybeStartPlayback() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp:969
    #10 0x7fa6f0b5426f in mozilla::MediaDecoderStateMachine::RunStateMachine() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp:2133
    #11 0x7fa6f0b60a80 in applyImpl<mozilla::MediaDecoderStateMachine, nsresult (mozilla::MediaDecoderStateMachine::*)()> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:671
    #12 0x7fa6f0b60a80 in apply<mozilla::MediaDecoderStateMachine, nsresult (mozilla::MediaDecoderStateMachine::*)()> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:677
    #13 0x7fa6f0b60a80 in nsRunnableMethodImpl<nsresult (mozilla::MediaDecoderStateMachine::*)(), true>::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:705
    #14 0x7fa6eba9c26a in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:192
    #15 0x7fa6eba7b6a0 in mozilla::TaskQueue::Runner::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/TaskQueue.cpp:171
    #16 0x7fa6eba90c83 in nsThreadPool::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadPool.cpp:228
    #17 0x7fa6eba912bc in non-virtual thunk to nsThreadPool::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/xpcom/threads/Unified_cpp_xpcom_threads0.cpp:242
    #18 0x7fa6eba8a240 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:994
    #19 0x7fa6ebb03fea in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290
    #20 0x7fa6ec7f7881 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:340
    #21 0x7fa6ec76ea3c in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:230
    #22 0x7fa6ec76ea3c in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:223
    #23 0x7fa6ec76ea3c in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:203
    #24 0x7fa6eba85c8e in nsThread::ThreadFunc(void*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:396
    #25 0x7fa701e603ef in _pt_root /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:216
    #26 0x7fa705382181 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8181)

0x61a000108c7c is located 4 bytes to the left of 1272-byte region [0x61a000108c80,0x61a000109178)
allocated by thread T4627 (MediaPl~back #1) here:
    #0 0x47245b in realloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:95
    #1 0x7fa6f431c4e1 in speex_realloc /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libspeex_resampler/src/resample.c:69
    #2 0x7fa6f431c4e1 in update_filter /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libspeex_resampler/src/resample.c:717
    #3 0x7fa6f431b306 in moz_speex_resampler_init_frac /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libspeex_resampler/src/resample.c:857
    #4 0x7fa6f431ac4a in moz_speex_resampler_init /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libspeex_resampler/src/resample.c:798
    #5 0x7fa6f0a73076 in RecreateResampler /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/AudioConverter.cpp:286
    #6 0x7fa6f0a73076 in mozilla::AudioConverter::AudioConverter(mozilla::AudioConfig const&, mozilla::AudioConfig const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/AudioConverter.cpp:36
    #7 0x7fa6f0db1a8d in MakeUnique<mozilla::AudioConverter, mozilla::AudioConfig, mozilla::AudioConfig> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:680
    #8 0x7fa6f0db1a8d in mozilla::media::DecodedAudioDataSink::NotifyAudioNeeded() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/mediasink/DecodedAudioDataSink.cpp:366
    #9 0x7fa6f0db098b in mozilla::media::DecodedAudioDataSink::Init(mozilla::media::MediaSink::PlaybackParams const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/mediasink/DecodedAudioDataSink.cpp:88
    #10 0x7fa6f0daeb4f in mozilla::media::AudioSinkWrapper::Start(long, mozilla::MediaInfo const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/mediasink/AudioSinkWrapper.cpp:191
    #11 0x7fa6f0dc1627 in mozilla::media::VideoSink::Start(long, mozilla::MediaInfo const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/mediasink/VideoSink.cpp:162
    #12 0x7fa6f0b4527a in mozilla::MediaDecoderStateMachine::StartMediaSink() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp:1703
    #13 0x7fa6f0b44e8a in mozilla::MediaDecoderStateMachine::MaybeStartPlayback() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp:969
    #14 0x7fa6f0b5426f in mozilla::MediaDecoderStateMachine::RunStateMachine() /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp:2133
    #15 0x7fa6f0b60a80 in applyImpl<mozilla::MediaDecoderStateMachine, nsresult (mozilla::MediaDecoderStateMachine::*)()> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:671
    #16 0x7fa6f0b60a80 in apply<mozilla::MediaDecoderStateMachine, nsresult (mozilla::MediaDecoderStateMachine::*)()> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:677
    #17 0x7fa6f0b60a80 in nsRunnableMethodImpl<nsresult (mozilla::MediaDecoderStateMachine::*)(), true>::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:705
    #18 0x7fa6eba9c26a in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:192
    #19 0x7fa6eba7b6a0 in mozilla::TaskQueue::Runner::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/TaskQueue.cpp:171
    #20 0x7fa6eba90c83 in nsThreadPool::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadPool.cpp:228
    #21 0x7fa6eba912bc in non-virtual thunk to nsThreadPool::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/xpcom/threads/Unified_cpp_xpcom_threads0.cpp:242
    #22 0x7fa6eba8a240 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:994
    #23 0x7fa6ebb03fea in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290
    #24 0x7fa6ec7f7881 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:340
    #25 0x7fa6ec76ea3c in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:230
    #26 0x7fa6ec76ea3c in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:223
    #27 0x7fa6ec76ea3c in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:203
    #28 0x7fa6eba85c8e in nsThread::ThreadFunc(void*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:396
    #29 0x7fa701e603ef in _pt_root /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:216
    #30 0x7fa705382181 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8181)

Thread T4627 (MediaPl~back #1) created by T0 here:
    #0 0x45ea55 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:175
    #1 0x7fa701e5cb40 in _PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:457
    #2 0x7fa701e5c6aa in PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:548
    #3 0x7fa6eba8741d in nsThread::Init() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:526
    #4 0x7fa6eba8dd1e in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadManager.cpp:253
    #5 0x7fa6eba8f72e in nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>&&, unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadPool.cpp:106
    #6 0x7fa6eba917c6 in nsThreadPool::Dispatch(already_AddRefed<nsIRunnable>&&, unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadPool.cpp:277
    #7 0x7fa6eba7a030 in mozilla::TaskQueue::DispatchLocked(nsCOMPtr<nsIRunnable>&, mozilla::TaskQueue::DispatchMode, mozilla::AbstractThread::DispatchFailureHandling, mozilla::AbstractThread::DispatchReason) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/TaskQueue.cpp:67
    #8 0x7fa6eba93531 in mozilla::TaskQueue::Dispatch(already_AddRefed<nsIRunnable>, mozilla::AbstractThread::DispatchFailureHandling, mozilla::AbstractThread::DispatchReason) /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/mozilla/TaskQueue.h:49
    #9 0x7fa6eba9bbcc in mozilla::AutoTaskDispatcher::DispatchTaskGroup(mozilla::UniquePtr<mozilla::AutoTaskDispatcher::PerThreadTaskGroup, mozilla::DefaultDelete<mozilla::AutoTaskDispatcher::PerThreadTaskGroup> >) /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:244
    #10 0x7fa6eba9cad1 in mozilla::AutoTaskDispatcher::~AutoTaskDispatcher() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:90
    #11 0x7fa6ebaa0991 in reset /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/mozilla/Maybe.h:373
    #12 0x7fa6ebaa0991 in mozilla::XPCOMThreadWrapper::FireTailDispatcher() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/AbstractThread.cpp:81
    #13 0x7fa6ebaa0b30 in applyImpl<mozilla::XPCOMThreadWrapper, void (mozilla::XPCOMThreadWrapper::*)()> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:671
    #14 0x7fa6ebaa0b30 in apply<mozilla::XPCOMThreadWrapper, void (mozilla::XPCOMThreadWrapper::*)()> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:677
    #15 0x7fa6ebaa0b30 in nsRunnableMethodImpl<void (mozilla::XPCOMThreadWrapper::*)(), true>::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:705
    #16 0x7fa6eb954b39 in mozilla::CycleCollectedJSRuntime::ProcessStableStateQueue() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/base/CycleCollectedJSRuntime.cpp:1327
    #17 0x7fa6ed339ef1 in XPCJSRuntime::AfterProcessTask(unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/js/xpconnect/src/XPCJSRuntime.cpp:3728
    #18 0x7fa6eba8a6ff in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:1009
    #19 0x7fa6ebb03fea in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290
    #20 0x7fa6ec7f660e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:98
    #21 0x7fa6ec76ea3c in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:230
    #22 0x7fa6ec76ea3c in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:223
    #23 0x7fa6ec76ea3c in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:203
    #24 0x7fa6f1d85207 in nsBaseAppShell::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/widget/nsBaseAppShell.cpp:156
    #25 0x7fa6f3c3efa8 in nsAppStartup::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:284
    #26 0x7fa6f3d3f1dc in XREMain::XRE_mainRun() /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4346
    #27 0x7fa6f3d404f8 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4450
    #28 0x7fa6f3d413de in XRE_main /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4558
    #29 0x48a793 in do_main /builds/slave/m-in-l64-asan-0000000000000000/build/src/browser/app/nsBrowserApp.cpp:220
    #30 0x48a793 in main /builds/slave/m-in-l64-asan-0000000000000000/build/src/browser/app/nsBrowserApp.cpp:360
    #31 0x7fa7043aaec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libspeex_resampler/src/resample.c:956 moz_speex_resampler_process_float
Shadow bytes around the buggy address:
  0x0c3480019130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480019140: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480019150: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480019160: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480019170: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c3480019180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
  0x0c3480019190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c34800191a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c34800191b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c34800191c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c34800191d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
Attached file Testcase (obsolete) —
Group: core-security → media-core-security
Keywords: sec-critical
Component: Audio/Video → Audio/Video: cubeb
Rank: 5
Priority: -- → P1
FYI
Flags: needinfo?(padenot)
Paul, this is probably an issue with the new resampler code.  Can you take this one?
Assignee: nobody → padenot
This is jya's new AudioConverter for media playback, that is using the same resampler external code underneath (the resampler from speex). Jean-Yves, let me know if you need anything.
Flags: needinfo?(padenot) → needinfo?(jyavenard)
Can you please retry with today's nightly?

the version you are trying and that crashes got backed out shortly after.

It's very likely that this bug is now fixed.
Flags: needinfo?(jyavenard) → needinfo?(cdiehl)
Assignee: padenot → jyavenard
Thanks for the correction, Paul.  Moving this to Playback.
Rank: 5
Component: Audio/Video: cubeb → Audio/Video: Playback
I am testing mozilla-inbound, the crash still appears on our cluster.
Flags: needinfo?(cdiehl)
(In reply to Christoph Diehl [:posidron] from comment #7)
> I am testing mozilla-inbound, the crash still appears on our cluster.

the code has changed significantly since, so the backtrace isn't very useful.

Could you provide an updated backtrace then?

thanks
I can't see a way you can enter this backtrace with the default preferences with that particular file.

The audio being tested is https://github.com/MozillaSecurity/fuzzdata/blob/master/samples/wav/wave_metadata.wav?raw=true if I read the zip file properly.

Its sampling rate is 44.1kHz as such, there can never be any resampling happening
https://hg.mozilla.org/mozilla-central/file/tip/dom/media/mediasink/DecodedAudioDataSink.cpp#l59

so we have outputrate == inputrate. no resampling.

Unless it's not this file:
https://github.com/MozillaSecurity/fuzzdata/blob/master/samples/wav/wave_metadata.wav
(appears identical to what's in data_1_output_Output.txt)

Are you setting particular preferences in your fuzzer by any chance? could you post what they are?
Can't really debug unless bug 1268718 and the samples provided allow to reproduce the problem
Depends on: 1268718
Attached file test_case.html
Attachment #8743628 - Attachment is obsolete: true
Attached audio test_case.wav
Thanks!
the sample attempts to downsample 1073742148Hz to 44100Hz

I'm guessing this is causing an overflow in speex.

We could simply refuse crazy sampling rate (say > 320kHz) but I'm guessing we should fix speex too.
Fix submitted upstream

MozReview-Commit-ID: 5RyIMI5qrvC
Additionally, indicate in clearer fashion where setting length can't fail.

MozReview-Commit-ID: LSZtCclqhK1
Attachment #8749992 - Flags: review?(gsquelart)
MozReview-Commit-ID: 5ulAivVkec5
Attachment #8749993 - Flags: review?(gsquelart)
Attachment #8749991 - Flags: review?(padenot)
Comment on attachment 8749992 [details] [diff] [review]
P2. Check for OOM. r?gerald

Review of attachment 8749992 [details] [diff] [review]:
-----------------------------------------------------------------

r+ as it seems you do want this, but just in case:

::: dom/media/AudioConverter.h
@@ +154,5 @@
>        return AudioDataBuffer<Format, Value>(Move(temp1));
>      }
>      frames = ProcessInternal(temp1.Data(), aBuffer.Data(), frames);
>      if (mIn.Rate() == mOut.Rate()) {
> +      MOZ_ALWAYS_TRUE(temp1.SetLength(FramesOutToSamples(frames)));

It seems a bit rude to assert here (and at line 179 below). Can't you exit gracefully?
Attachment #8749992 - Flags: review?(gsquelart) → review+
Attachment #8749993 - Flags: review?(gsquelart) → review+
(In reply to Gerald Squelart [:gerald] from comment #18)
> Comment on attachment 8749992 [details] [diff] [review]
> P2. Check for OOM. r?gerald
>      frames = ProcessInternal(temp1.Data(), aBuffer.Data(), frames);
> >      if (mIn.Rate() == mOut.Rate()) {
> > +      MOZ_ALWAYS_TRUE(temp1.SetLength(FramesOutToSamples(frames)));
> 
> It seems a bit rude to assert here (and at line 179 below). Can't you exit
> gracefully?

because it can't happen. Those SetLength calls are always with values that are less than or equal to the actual allocated size. As such it will always return true. So just to show that we are testing the return value, and that really, it will always succeed.
This is similar to the solution used for static analysis working with nsFallibleTArray the operations were always checked.
Duplicate of this bug: 1267711
Fix submitted upstream

MozReview-Commit-ID: 5RyIMI5qrvC
Attachment #8750130 - Flags: review?(padenot)
Attachment #8749991 - Attachment is obsolete: true
Attachment #8749991 - Flags: review?(padenot)
(In reply to Jean-Yves Avenard [:jya] from comment #21)
> Created attachment 8750130 [details] [diff] [review]
> P1. Fix overflow is speexdsp. r?padenot
> 
> Fix submitted upstream

My comment was lost in git-bz; so here it is again.

Upstream included a fix for ensuring some operations didn't overflow. Though incomplete as the operation could still overflow (the fix just reduced the likelihood of overflow). I've included the fix here and completed it. I used the name for the function that was used upstream (_muldiv)
Attachment #8750130 - Flags: review?(padenot) → review+
Comment on attachment 8750130 [details] [diff] [review]
P1. Fix overflow is speexdsp. r?padenot

[Security approval request comment]
How easily could an exploit be constructed based on the patch? Causing the issue is rather simple, exploiting it is a different story.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? The error is rather generic.

Which older supported branches are affected by this flaw? Ever since we used speexdsp ; the issue was pointed out following bug 1264199, but it will be just as present in other user of speex resampler such as cubeb.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? P1 patch should apply on all current release. P2 and P3 prevent the overflow from occurring in the first place and is only necessary in 49.

How likely is this patch to cause regressions; how much testing does it need? not likely at all
Attachment #8750130 - Flags: sec-approval?
Has anyone worked through the consequence of that vulnerability? Does it cause a read overflow, a write overflow, arbitrary code execution, ...? I'm asked since we need to fix this upstream (I'm the author of the code, though not the current maintainer). Also, it just occurred to me that one of the main reason for this bug is a difference in the assumptions I made when writing the code compared to how Firefox uses the code. The original assumption were that the rates themselves were set by the developer and hence were "trusted" values, which is not the case for Firefox. Is there other code/libraries that Firefox reuses like this. Maybe some of these would need to be checked for this kind of differences in assumptions about what's trusted.
The stack indicates a write of 4 bytes that eases exploit development for arbitrary code execution a lot.
sec-approval+ for trunk. 

The answers in comment 23 don't make it clear to me how far back this issue actually goes. Assuming a long way, we'll want this address on Aurora, Beta, and ESR45 and patches for those branches nominated shortly after it goes into trunk.
Attachment #8750130 - Flags: sec-approval? → sec-approval+
(In reply to Jean-Marc Valin (:jmspeex) from comment #24)
> Has anyone worked through the consequence of that vulnerability? Does it
> cause a read overflow, a write overflow, arbitrary code execution, ...? I'm

in the example attached, it causes a write at offset at offset -1. So 4 bytes before the actual memory allocation.
the overflow could occur in various places so that may not always be the problem.

On top of the fix to speexdsp itself, P3 applies extra sanity check so that rate > 640kHz aren't allowed.


> asked since we need to fix this upstream (I'm the author of the code, though
> not the current maintainer). Also, it just occurred to me that one of the
> main reason for this bug is a difference in the assumptions I made when
> writing the code compared to how Firefox uses the code. The original
> assumption were that the rates themselves were set by the developer and
> hence were "trusted" values, which is not the case for Firefox. Is there

question is what's a trusted value?
the input rate is often going to be the rate set in the metadata of the file being played no?
So I don't know how widespread the vulnerability will be.
(In reply to Al Billings [:abillings] from comment #26)
> sec-approval+ for trunk. 
> 
> The answers in comment 23 don't make it clear to me how far back this issue
> actually goes. Assuming a long way, we'll want this address on Aurora, Beta,
> and ESR45 and patches for those branches nominated shortly after it goes
> into trunk.

The time when it got discovered on our cluster was: Wed, 20 Apr 2016
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6655d9a086b8850cfe8fb16f11af87f879e59be
I did never see this crash before and we used the same samples.
IIUC, the speex resampler is only used by cubeb on Windows; for which we don't have asan testing right?
On Linux, pulse limits the sampling rate to 384kHz
libcubeb has validated the stream sample rate since 2013, before it even used the resampler from libspeex.
Keywords: checkin-needed
Group: media-core-security → core-security-release
(In reply to Matthew Gregan [:kinetik] from comment #31)
> libcubeb has validated the stream sample rate since 2013, before it even
> used the resampler from libspeex.

Does this mean we can forget about your comment 29 and use comment 28 to say that Firefox 47 and below are unaffected?
Flags: needinfo?(jyavenard)
Sounds like it.
Flags: needinfo?(jyavenard)
Marking this unaffected for 46/47 and esr45, and tracking for 48, based on comment 28 and 35. 
Looks like we may want to uplift this to 48.
Shall we uplift to 48?  See comment 36
Flags: needinfo?(jyavenard)
Comment on attachment 8750130 [details] [diff] [review]
P1. Fix overflow is speexdsp. r?padenot

Approval Request Comment
[Feature/regressing bug #]:1266260
[User impact if declined]: not sure if it can be triggered in all versions. But out of bound memory access. 
[Describe test coverage new/current, TreeHerder]: in central for a while
[Risks and why]: low. Checking for overflow
[String/UUID change made/needed]:none
Attachment #8750130 - Flags: approval-mozilla-aurora?
Whiteboard: take bug 1274083 also when landing on Fx48
(In reply to Jean-Yves Avenard [:jya] from comment #38)
> Approval Request Comment
> [Feature/regressing bug #]:1266260

This _is_ bug 1266260, the regressing bug looks like bug 1264199

> [Risks and why]: low. Checking for overflow

Another risk is not checking in the security bug that was a regression from this fix (bug 1274083). I'd like to see that we're ready to go with a branch approval request there before approving this one.
Blocks: 1264199
(In reply to Daniel Veditz [:dveditz] from comment #39)
> (In reply to Jean-Yves Avenard [:jya] from comment #38)
> > Approval Request Comment
> > [Feature/regressing bug #]:1266260
> 
> This _is_ bug 1266260, the regressing bug looks like bug 1264199
> 
> > [Risks and why]: low. Checking for overflow
> 
> Another risk is not checking in the security bug that was a regression from
> this fix (bug 1274083). I'd like to see that we're ready to go with a branch
> approval request there before approving this one.

Bug 1274083 shows that the vulnerability existed before bug 1264199; since speexdsp resampler started to be used.

So maybe we should reconsider uplifting to other branches to, in particular 45.
Flags: needinfo?(jyavenard)
Comment on attachment 8750130 [details] [diff] [review]
P1. Fix overflow is speexdsp. r?padenot

This missed the merge, so it's fixed in aurora 49 already and the approval flag should be for beta.
Attachment #8750130 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 8750130 [details] [diff] [review]
P1. Fix overflow is speexdsp. r?padenot

Fix a sec-critical issue with a testcase, in nightly for several days.
Should be in 48 beta 3.
Attachment #8750130 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
does not apply to beta cleanly:

grafting 344396:afe186de263f "Bug 1266260: P1. Fix overflow is speexdsp. r=padenot"
merging media/libspeex_resampler/src/resample.c
merging media/libspeex_resampler/update.sh
warning: conflicts while merging media/libspeex_resampler/update.sh! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(jyavenard)
gerald,since jya is on pto, could you take a look ?
Flags: needinfo?(gsquelart)
Rebased part 1.
Flags: needinfo?(jyavenard)
Attachment #8764171 - Flags: review+
Rebased part 2.
Attachment #8764172 - Flags: review+
Rebased part 3 of 3.
Flags: needinfo?(gsquelart)
Attachment #8764173 - Flags: review+
Flags: qe-verify+
Group: core-security-release
Using this build: http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-beta-linux64-asan/1468498182/firefox-48.0.en-US.linux-x86_64-asan.tar.bz2
the tab with the testcase.html is loading forever.
Thoughts?
Flags: needinfo?(jyavenard)
there are more things that went in to ensure that we couldn't play the file. 48 only received the vulnerability fix.

in later version, the file is outright refused for nonsensical metadata.
Flags: needinfo?(jyavenard)
Reproduced on FX 48.0a1 (2016-04-20) asan build, Ubuntu 14.04 x64.
Verified fixed FX 48.0.2, 49.0b5 asan builds.
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.