Closed Bug 998711 Opened 6 years ago Closed 6 years ago

Heap-buffer-overflow in speex_resampler_process_float

Categories

(Core :: Web Audio, defect)

x86_64
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
firefox30 --- unaffected
firefox31 --- fixed
firefox32 --- fixed
firefox-esr24 --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- fixed

People

(Reporter: inferno, Assigned: padenot)

References

Details

(Keywords: sec-moderate, Whiteboard: [qa-])

Attachments

(2 files, 1 obsolete file)

==12513==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6050004be820 at pc 0x7f04190da560 bp 0x7f03edb41cd0 sp 0x7f03edb41cc8
READ of size 4 at 0x6050004be820 thread T31 (Media Decode #2)
    #0 0x7f04190da55f in speex_resampler_process_float media/libspeex_resampler/src/resample.c:888
    #1 0x7f0416db5115 in void mozilla::AudioSegment::Resample<float>(SpeexResamplerState_*, unsigned int, unsigned int) content/media/AudioSegment.h:200
    #2 0x7f0416db4a5a in mozilla::AudioSegment::ResampleChunks(SpeexResamplerState_*) content/media/AudioSegment.cpp:140
    #3 0x7f0416df86fb in mozilla::SourceMediaStream::AppendToTrack(int, mozilla::MediaSegment*, mozilla::MediaSegment*) content/media/MediaStreamGraph.cpp:2319
    #4 0x7f0416df5955 in mozilla::MediaDecoderStateMachine::SendStreamData() content/media/MediaDecoderStateMachine.cpp:404
    #5 0x7f0416dfd2f9 in mozilla::MediaDecoderStateMachine::DecodeAudio() content/media/MediaDecoderStateMachine.cpp:684
    #6 0x7f0416e56990 in nsRunnableMethodImpl<void (mozilla::MediaDecoderStateMachine::*)(), void, true>::Run() objdir-ff-asan/content/media/../../dist/include/nsThreadUtils.h:387
    #7 0x7f0416e40db0 in mozilla::MediaTaskQueue::Runner::Run() content/media/MediaTaskQueue.cpp:127
    #8 0x7f04131455da in nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp:211
    #9 0x7f04131458dc in non-virtual thunk to nsThreadPool::Run() objdir-ff-asan/xpcom/threads/Unified_cpp_xpcom_threads0.cpp:225
    #10 0x7f0413140020 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:699
    #11 0x7f0413004fda in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:263
    #12 0x7f0413915696 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:307
    #13 0x7f04138bd3b0 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:226
    #14 0x7f041313ce05 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:311
    #15 0x7f041f3a0d95 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:212
    #16 0x7f04228c8f6d in start_thread /build/buildd/eglibc-2.17/nptl/pthread_create.c:311
0x6050004be821 is located 0 bytes to the right of 1-byte region [0x6050004be820,0x6050004be821)
allocated by thread T31 (Media Decode #2) here:
    #0 0x473e91 in calloc _asan_rtl_
    #1 0x7f04190d73fa in speex_resampler_init_frac media/libspeex_resampler/src/resample.c:68
    #2 0x7f04190d707a in speex_resampler_init media/libspeex_resampler/src/resample.c:756
    #3 0x7f0416e39499 in mozilla::SourceMediaStream::ResampleAudioToGraphSampleRate(mozilla::SourceMediaStream::TrackData*, mozilla::MediaSegment*) content/media/MediaStreamGraph.cpp:2288
    #4 0x7f0416df86fb in mozilla::SourceMediaStream::AppendToTrack(int, mozilla::MediaSegment*, mozilla::MediaSegment*) content/media/MediaStreamGraph.cpp:2319
    #5 0x7f0416df5955 in mozilla::MediaDecoderStateMachine::SendStreamData() content/media/MediaDecoderStateMachine.cpp:404
    #6 0x7f0416dfd2f9 in mozilla::MediaDecoderStateMachine::DecodeAudio() content/media/MediaDecoderStateMachine.cpp:684
    #7 0x7f0416e56990 in nsRunnableMethodImpl<void (mozilla::MediaDecoderStateMachine::*)(), void, true>::Run() objdir-ff-asan/content/media/../../dist/include/nsThreadUtils.h:387
    #8 0x7f0416e40db0 in mozilla::MediaTaskQueue::Runner::Run() content/media/MediaTaskQueue.cpp:127
    #9 0x7f04131455da in nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp:211
    #10 0x7f04131458dc in non-virtual thunk to nsThreadPool::Run() objdir-ff-asan/xpcom/threads/Unified_cpp_xpcom_threads0.cpp:225
    #11 0x7f0413140020 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:699
    #12 0x7f0413004fda in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:263
    #13 0x7f0413915696 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:307
    #14 0x7f04138bd3b0 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:226
    #15 0x7f041313ce05 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:311
    #16 0x7f041f3a0d95 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:212
    #17 0x7f04228c8f6d in start_thread /build/buildd/eglibc-2.17/nptl/pthread_create.c:311

Thread T31 (Media Decode #2) created by T30 (Media Decode #1) here:
    #0 0x4602f5 in __interceptor_pthread_create _asan_rtl_
    #1 0x7f041f39d71d in _PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7f041f39d29a in PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7f041313e19b in nsThread::Init() xpcom/threads/nsThread.cpp:384
    #4 0x7f041314361c in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) xpcom/threads/nsThreadManager.cpp:232
    #5 0x7f04131449ca in nsThreadPool::PutEvent(nsIRunnable*) xpcom/threads/nsThreadPool.cpp:95
    #6 0x7f0413145cf6 in nsThreadPool::Dispatch(nsIRunnable*, unsigned int) xpcom/threads/nsThreadPool.cpp:249
    #7 0x7f0416e4107a in mozilla::MediaTaskQueue::Runner::Run() content/media/MediaTaskQueue.cpp:158
    #8 0x7f04131455da in nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp:211
    #9 0x7f04131458dc in non-virtual thunk to nsThreadPool::Run() objdir-ff-asan/xpcom/threads/Unified_cpp_xpcom_threads0.cpp:225
    #10 0x7f0413140020 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:699
    #11 0x7f0413004fda in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:263
    #12 0x7f0413915696 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:307
    #13 0x7f04138bd3b0 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:226
    #14 0x7f041313ce05 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:311
    #15 0x7f041f3a0d95 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:212
    #16 0x7f04228c8f6d in start_thread /build/buildd/eglibc-2.17/nptl/pthread_create.c:311

Thread T30 (Media Decode #1) created by T29 (Media S~hine #1) here:
    #0 0x4602f5 in __interceptor_pthread_create _asan_rtl_
    #1 0x7f041f39d71d in _PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7f041f39d29a in PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7f041313e19b in nsThread::Init() xpcom/threads/nsThread.cpp:384
    #4 0x7f041314361c in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) xpcom/threads/nsThreadManager.cpp:232
    #5 0x7f04131449ca in nsThreadPool::PutEvent(nsIRunnable*) xpcom/threads/nsThreadPool.cpp:95
    #6 0x7f0413145cf6 in nsThreadPool::Dispatch(nsIRunnable*, unsigned int) xpcom/threads/nsThreadPool.cpp:249
    #7 0x7f0416e06127 in mozilla::MediaTaskQueue::Dispatch(nsIRunnable*) content/media/MediaTaskQueue.cpp:41
    #8 0x7f0416e0cbb3 in mozilla::MediaDecoderStateMachine::RunStateMachine() content/media/MediaDecoderStateMachine.cpp:1487
    #9 0x7f0416e10f53 in mozilla::TimeoutExpired(nsITimer*, void*) content/media/MediaDecoderStateMachine.cpp:2687
    #10 0x7f0413148914 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:551
    #11 0x7f0413148f9e in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:635
    #12 0x7f04131455da in nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp:211
    #13 0x7f04131458dc in non-virtual thunk to nsThreadPool::Run() objdir-ff-asan/xpcom/threads/Unified_cpp_xpcom_threads0.cpp:225
    #14 0x7f0413140020 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:699
    #15 0x7f0413004fda in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:263
    #16 0x7f0413915696 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:307
    #17 0x7f04138bd3b0 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:226
    #18 0x7f041313ce05 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:311
    #19 0x7f041f3a0d95 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:212
    #20 0x7f04228c8f6d in start_thread /build/buildd/eglibc-2.17/nptl/pthread_create.c:311

Thread T29 (Media S~hine #1) created by T9 (Timer) here:
    #0 0x4602f5 in __interceptor_pthread_create _asan_rtl_
    #1 0x7f041f39d71d in _PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7f041f39d29a in PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7f041313e19b in nsThread::Init() xpcom/threads/nsThread.cpp:384
    #4 0x7f041314361c in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) xpcom/threads/nsThreadManager.cpp:232
    #5 0x7f04131449ca in nsThreadPool::PutEvent(nsIRunnable*) xpcom/threads/nsThreadPool.cpp:95
    #6 0x7f0413145cf6 in nsThreadPool::Dispatch(nsIRunnable*, unsigned int) xpcom/threads/nsThreadPool.cpp:249
    #7 0x7f041313734a in nsTimerImpl::PostTimerEvent() xpcom/threads/nsTimerImpl.cpp:677
    #8 0x7f041313693b in TimerThread::Run() xpcom/threads/TimerThread.cpp:256
    #9 0x7f0413140020 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:699
    #10 0x7f0413004fda in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:263
    #11 0x7f0413915696 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:307
    #12 0x7f04138bd3b0 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:226
    #13 0x7f041313ce05 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:311
    #14 0x7f041f3a0d95 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:212
    #15 0x7f04228c8f6d in start_thread /build/buildd/eglibc-2.17/nptl/pthread_create.c:311

Thread T9 (Timer) created by T8 (Cache2 I/O) here:
    #0 0x4602f5 in __interceptor_pthread_create _asan_rtl_
    #1 0x7f041f39d71d in _PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7f041f39d29a in PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7f041313e19b in nsThread::Init() xpcom/threads/nsThread.cpp:384
    #4 0x7f041314361c in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) xpcom/threads/nsThreadManager.cpp:232
    #5 0x7f041300451c in NS_NewThread(nsIThread**, nsIRunnable*, unsigned int) xpcom/glue/nsThreadUtils.cpp:67
    #6 0x7f0413135a2c in TimerThread::Init() xpcom/threads/TimerThread.cpp:92
    #7 0x7f04131471b6 in nsTimerImpl::InitCommon(unsigned int, unsigned int) xpcom/threads/nsTimerImpl.cpp:327
    #8 0x7f041347ae2a in mozilla::net::CacheFileIOManager::StartRemovingTrash() netwerk/cache2/CacheFileIOManager.cpp:2887
    #9 0x7f041346ee9b in mozilla::net::CacheFileIOManager::CreateCacheTree() netwerk/cache2/CacheFileIOManager.cpp:3404
    #10 0x7f041347049a in mozilla::net::CacheFileIOManager::OpenSpecialFileInternal(nsACString_internal const&, unsigned int, mozilla::net::CacheFileHandle**) netwerk/cache2/CacheFileIOManager.cpp:1679
    #11 0x7f04134834a7 in mozilla::net::OpenFileEvent::Run() netwerk/cache2/CacheFileIOManager.cpp:594
    #12 0x7f04134d02aa in mozilla::net::CacheIOThread::LoopOneLevel(unsigned int) netwerk/cache2/CacheIOThread.cpp:281
    #13 0x7f04134cfa4f in mozilla::net::CacheIOThread::ThreadFunc() netwerk/cache2/CacheIOThread.cpp:211
    #14 0x7f04134cebe3 in mozilla::net::CacheIOThread::ThreadFunc(void*) netwerk/cache2/CacheIOThread.cpp:158
    #15 0x7f041f3a0d95 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:212
    #16 0x7f04228c8f6d in start_thread /build/buildd/eglibc-2.17/nptl/pthread_create.c:311

Thread T8 (Cache2 I/O) created by T0 here:
    #0 0x4602f5 in __interceptor_pthread_create _asan_rtl_
    #1 0x7f041f39d71d in _PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7f041f39d29a in PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7f04134ceb86 in mozilla::net::CacheIOThread::Init() netwerk/cache2/CacheIOThread.cpp:46
    #4 0x7f04134693af in mozilla::net::CacheFileIOManager::InitInternal() netwerk/cache2/CacheFileIOManager.cpp:1169
    #5 0x7f0413469267 in mozilla::net::CacheFileIOManager::Init() netwerk/cache2/CacheFileIOManager.cpp:1155
    #6 0x7f04134d1a71 in mozilla::net::CacheObserver::Observe(nsISupports*, char const*, char16_t const*) netwerk/cache2/CacheObserver.cpp:404
    #7 0x7f04130a1c39 in nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*) xpcom/ds/nsObserverList.cpp:96
    #8 0x7f041885bad5 in nsXREDirProvider::DoStartup() toolkit/xre/nsXREDirProvider.cpp:804
    #9 0x7f041883c88f in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3905
    #10 0x7f041883e29d in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:4088
    #11 0x7f041883f0ed in XRE_main toolkit/xre/nsAppRunner.cpp:4300
    #12 0x48c6e7 in main browser/app/nsBrowserApp.cpp:282
    #13 0x7f04218f0de4 in __libc_start_main /build/buildd/eglibc-2.17/csu/libc-start.c:260

SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 ??
Shadow bytes around the buggy address:
  0x0c0a8008fcb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0a8008fcc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0a8008fcd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0a8008fce0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0a8008fcf0: 01 fa fa fa fa fa fa fa fa fa 01 fa fa fa fa fa
=>0x0c0a8008fd00: fa fa fa fa[01]fa fa fa fa fa fa fa fa fa 01 fa
  0x0c0a8008fd10: fa fa fa fa fa fa fa fa fd fd fa fa fa fa fa fa
  0x0c0a8008fd20: fa fa fd fd fa fa fa fa fa fa fa fa fd fd fa fa
  0x0c0a8008fd30: fa fa fa fa fa fa fd fd fa fa fa fa fa fa fa fa
  0x0c0a8008fd40: fd fd fa fa fa fa fa fa fa fa fd fd fa fa fa fa
  0x0c0a8008fd50: fa fa fa fa fd fd fa fa fa fa fa fa fa fa 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
==12513==ABORTING
This does not reproduce on trunk, crash stack looks similar to https://bugzilla.mozilla.org/show_bug.cgi?id=991504. My build was 2-3 days old, maybe cc the developer fixing that bug to verify this one.
Is this a dupe?
Flags: needinfo?(paul)
(In reply to Abhishek Arya from comment #0)
> READ of size 4 at 0x6050004be820 thread T31 (Media Decode #2)
>     #0 0x7f04190da55f in speex_resampler_process_float
> media/libspeex_resampler/src/resample.c:888

http://hg.mozilla.org/mozilla-central/annotate/33ca5e321046/media/libspeex_resampler/src/resample.c#l888

> 0x6050004be821 is located 0 bytes to the right of 1-byte region
> [0x6050004be820,0x6050004be821)
> allocated by thread T31 (Media Decode #2) here:
>     #0 0x473e91 in calloc _asan_rtl_
>     #1 0x7f04190d73fa in speex_resampler_init_frac
> media/libspeex_resampler/src/resample.c:68
>     #2 0x7f04190d707a in speex_resampler_init
> media/libspeex_resampler/src/resample.c:756

Looks like the resampler may have been initialized with zero channels.
(In reply to Andrew McCreight [:mccr8] from comment #2)
> Is this a dupe?

Please ignore my comment 1. It still reproduces on trunk. The problem is testcase is very very flaky.
On windows machine, it is reliably crashing in debugger on trunk build.

static int speex_resampler_process_native(SpeexResamplerState *st, spx_uint32_t channel_index, spx_uint32_t *in_len, spx_word16_t *out, spx_uint32_t *out_len)
{
........

   for(j=0;j<N-1;++j)
     mem[j] = mem[j+ilen]; // crashes here.
(In reply to Karl Tomlinson (:karlt, back May 5) from comment #3)
> (In reply to Abhishek Arya from comment #0)
> > READ of size 4 at 0x6050004be820 thread T31 (Media Decode #2)
> >     #0 0x7f04190da55f in speex_resampler_process_float
> > media/libspeex_resampler/src/resample.c:888
> 
> http://hg.mozilla.org/mozilla-central/annotate/33ca5e321046/media/
> libspeex_resampler/src/resample.c#l888
> 
> > 0x6050004be821 is located 0 bytes to the right of 1-byte region
> > [0x6050004be820,0x6050004be821)
> > allocated by thread T31 (Media Decode #2) here:
> >     #0 0x473e91 in calloc _asan_rtl_
> >     #1 0x7f04190d73fa in speex_resampler_init_frac
> > media/libspeex_resampler/src/resample.c:68
> >     #2 0x7f04190d707a in speex_resampler_init
> > media/libspeex_resampler/src/resample.c:756
> 
> Looks like the resampler may have been initialized with zero channels.

Yes, nb_channels is 0
   st->last_sample = (spx_int32_t*)speex_alloc(nb_channels*sizeof(spx_int32_t));
   st->magic_samples = (spx_uint32_t*)speex_alloc(nb_channels*sizeof(spx_uint32_t));
   st->samp_frac_num = (spx_uint32_t*)speex_alloc(nb_channels*sizeof(spx_uint32_t));

And we are later manipulating all these data structures.

static int speex_resampler_process_native(SpeexResamplerState *st, spx_uint32_t channel_index, spx_uint32_t *in_len, spx_word16_t *out, spx_uint32_t *out_len)
{
...........
   st->last_sample[channel_index] -= *in_len;
   
   ilen = *in_len;

   for(j=0;j<N-1;++j)
     mem[j] = mem[j+ilen];


 	gkmedias.dll!resampler_basic_direct_single(SpeexResamplerState_ * st, unsigned int channel_index, const float * in, unsigned int * in_len, float * out, unsigned int * out_len)  Line 365 + 0x20 bytes	C
>	gkmedias.dll!speex_resampler_process_native(SpeexResamplerState_ * st, unsigned int channel_index, unsigned int * in_len, float * out, unsigned int * out_len)  Line 839 + 0x20 bytes	C
 	gkmedias.dll!speex_resampler_process_float(SpeexResamplerState_ * st, unsigned int channel_index, const float * in, unsigned int * in_len, float * out, unsigned int * out_len)  Line 902 + 0x19 bytes	C
 	xul.dll!mozilla::dom::WebAudioUtils::SpeexResamplerProcess(SpeexResamplerState_ * aResampler, unsigned int aChannel, const float * aIn, unsigned int * aInLen, float * aOut, unsigned int * aOutLen)  Line 69 + 0x1d bytes	C++
 	xul.dll!mozilla::AudioSegment::Resample<float>(SpeexResamplerState_ * aResampler, unsigned int aInRate, unsigned int aOutRate)  Line 202 + 0x1d bytes	C++
 	xul.dll!mozilla::AudioSegment::ResampleChunks(SpeexResamplerState_ * aResampler)  Line 141	C++
 	xul.dll!mozilla::SourceMediaStream::ResampleAudioToGraphSampleRate(mozilla::SourceMediaStream::TrackData * aTrackData, mozilla::MediaSegment * aSegment)  Line 2302	C++
 	xul.dll!mozilla::SourceMediaStream::AppendToTrack(int aID, mozilla::MediaSegment * aSegment, mozilla::MediaSegment * aRawSegment)  Line 2326	C++
 	xul.dll!mozilla::MediaDecoderStateMachine::SendStreamData()  Line 418	C++
 	xul.dll!mozilla::MediaDecoderStateMachine::DecodeAudio()  Line 697	C++
 	xul.dll!nsRunnableMethodImpl<void (__thiscall mozilla::MediaDecoderStateMachine::*)(void),void,1>::Run()  Line 388	C++
 	xul.dll!mozilla::MediaTaskQueue::Runner::Run()  Line 134	C++
 	xul.dll!nsThreadPool::Run()  Line 213	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait, bool * result)  Line 701 + 0x19 bytes	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * thread, bool mayWait)  Line 263 + 0x17 bytes	C++
 	xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate * aDelegate)  Line 336 + 0xe bytes	C++
 	xul.dll!MessageLoop::RunInternal()  Line 227	C++
 	xul.dll!MessageLoop::RunHandler()  Line 220	C++
 	xul.dll!MessageLoop::Run()  Line 194	C++
 	xul.dll!nsThread::ThreadFunc(void * arg)  Line 318	C++
 	nss3.dll!_PR_NativeRunThread(void * arg)  Line 397 + 0xf bytes	C
 	nss3.dll!pr_root(void * arg)  Line 90 + 0xf bytes	C
 	msvcr100d.dll!__beginthreadex()  + 0x243 bytes	
 	msvcr100d.dll!__beginthreadex()  + 0x1d4 bytes	
 	kernel32.dll!@BaseThreadInitThunk@12()  + 0x12 bytes	
 	ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes	
 	ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes
Attached patch r= (obsolete) — Splinter Review
It can be that the chunks have a channel count of zero, so we create the
resampler with 0 channels, which is wrong.

This implements a more robust channel count, and delays the creation of the
resampler if the whole MediaSegment is silent.
Attachment #8412586 - Flags: review?(rjesup)
Assignee: nobody → paul
Status: NEW → ASSIGNED
(clearing needinfo because I provided a patch instead)
Flags: needinfo?(paul)
Comment on attachment 8412586 [details] [diff] [review]
r=

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

::: content/media/AudioSegment.h
@@ +265,5 @@
>  
>    int ChannelCount() {
>      NS_WARN_IF_FALSE(!mChunks.IsEmpty(),
>          "Cannot query channel count on a AudioSegment with no chunks.");
> +    // Find the first chunk that has non-zero channels. The chunk that have zero

"The chunk that have" -> "A chunk that has"

::: content/media/MediaStreamGraph.cpp
@@ +2275,5 @@
>    }
>    AudioSegment* segment = static_cast<AudioSegment*>(aSegment);
> +  int channels = segment->ChannelCount();
> +  // If this segment is just silence, we delay instanciating the resampler.
> +  if (!aTrackData->mResampler && channels != 0) {

Don't count the channels all the time...

if (!aTrackData->mResampler) {
  int channels = segment->ChannelCount();
  if (!channels) {
     return;
  }
   ...

This also avoids calling ResampleChunks with a null resampler, which will likely crash as it doesn't null-check.  

Please also add a return:
  if (!state) {
    return;
  }
  aTrackData->mResampler.own(state);

to avoid ReSampleChunks if speex_resampler_init() failed.
Attachment #8412586 - Flags: review?(rjesup) → review-
Attached patch r=Splinter Review
Addressed review comments.
Attachment #8413785 - Flags: review?(rjesup)
Attachment #8412586 - Attachment is obsolete: true
Comment on attachment 8413785 [details] [diff] [review]
r=

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

::: content/media/MediaStreamGraph.cpp
@@ +2110,5 @@
>    AudioSegment* segment = static_cast<AudioSegment*>(aSegment);
>    if (!aTrackData->mResampler) {
>      int channels = segment->ChannelCount();
> +
> +    // If this segment is just silence, we delay instanciating the resampler.

instantiating

@@ +2121,5 @@
> +      if (state) {
> +        aTrackData->mResampler.own(state);
> +      } else {
> +        return;
> +      }

Style nit:

if (!state) {
  return;
}
aTrackData->mResampler.own(state);
Attachment #8413785 - Flags: review?(rjesup) → review+
Comment on attachment 8413785 [details] [diff] [review]
r=

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easy, the crash occurs because we read bytes not allocated.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Kind of, the comments can be checked-in separately.

Which older supported branches are affected by this flaw?
Current master (31)

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
N/A

How likely is this patch to cause regressions; how much testing does it need?
The cause is super obvious, and the fix straightforward.
Attachment #8413785 - Flags: sec-approval?
Note: this will need Aurora approval, since master should be 32 now by the time this lands (likely already)
We need to figure out a security rating for this but then it should be able to go in on Trunk (and then Aurora) with no issues.
Making sec-moderate on discussion with Dveditz. This means that this doesn't need sec-approval and can just go in.
Keywords: sec-moderate
Attachment #8413785 - Flags: sec-approval?
Comment on attachment 8413785 [details] [diff] [review]
r=

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 982490
User impact if declined: crash
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): low, alternative is to live with a crash  
String or IDL/UUID changes made by this patch: none
Attachment #8413785 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/8a69388a1a21
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Attachment #8413785 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Matt, do you think this needs to be verified? Thanks!
Flags: needinfo?(mwobensmith)
(In reply to Liz Henry :lizzard from comment #20)
> Matt, do you think this needs to be verified? Thanks!

Yes, indeed. It will require an ASan build.
Flags: needinfo?(mwobensmith)
Otilia can you please have someone on your team test this?
Keywords: verifyme
Unable to reproduce on ASan build from 2014-04-18, as well as post-checkin on a build from 2014-07-02. So, while we confirm that we don't see a crash, we still have to mark [qa-] with regards to verification.

Abhishek, if you can confirm that this is fixed for you, that would be helpful in any case.
Keywords: verifyme
Whiteboard: [qa-]
(In reply to Matt Wobensmith from comment #24)
> Unable to reproduce on ASan build from 2014-04-18, as well as post-checkin
> on a build from 2014-07-02. So, while we confirm that we don't see a crash,
> we still have to mark [qa-] with regards to verification.
> 
> Abhishek, if you can confirm that this is fixed for you, that would be
> helpful in any case.

Does not reproduce anymore on trunk.
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.