945618 - Buffer overflow in OPUS tansig_approx

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Abhishek Arya]

Attachment: test.html

=================================================================
==19033==ERROR: AddressSanitizer: SEGV on unknown address 0x7f5ce04bcce0 (pc 0x7f5ed67f07af sp 0x7f5e8ed42e60 bp 0x7f5e8ed430d0 T55)
AddressSanitizer can not provide additional info.
    #0 0x7f5ed67f07ae in tansig_approx media/libopus/src/mlp.c:81
    #1 0x435fdb in __asan::ASAN_OnSIGSEGV(int, siginfo*, void*) _asan_rtl_
    #2 0x7f5ed5086766 in nsProfileLock::FatalSignalHandler(int, siginfo*, void*) profile/dirserviceprovider/src/nsProfileLock.cpp:185
    #3 0x7f5edbcb3deb in AsmJSFaultHandler(int, siginfo*, void*) js/src/jit/AsmJSSignalHandlers.cpp:988
    #4 0x7f5eea859caf in
    #5 0x7f5ed67f07ae in tansig_approx media/libopus/src/mlp.c:81
    #6 0x7f5ed67ef72a in mlp_process media/libopus/src/mlp.c:100:15
    #7 0x7f5ed67e5f58 in tonality_analysis media/libopus/src/analysis.c:480
    #8 0x7f5ed67edaac in run_analysis media/libopus/src/analysis.c:627
    #9 0x7f5ed673995c in opus_encode_float media/libopus/src/opus_encoder.c:1924
    #10 0x7f5ecf18a5cf in mozilla::OpusTrackEncoder::GetEncodedTrack(mozilla::EncodedFrameContainer&) content/media/encoder/OpusTrackEncoder.cpp:348
    #11 0x7f5ecf182270 in mozilla::MediaEncoder::GetEncodedData(nsTArray<nsTArray<unsigned char> >*, nsAString_internal&) content/media/encoder/MediaEncoder.cpp:193
    #12 0x7f5ecf15d445 in mozilla::dom::MediaRecorder::Session::Extract() content/media/MediaRecorder.cpp:253
    #13 0x7f5ecf15cbeb in mozilla::dom::MediaRecorder::Session::ExtractRunnable::Run() content/media/MediaRecorder.cpp:108
    #14 0x7f5ec1bb57e3 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:612
    #15 0x7f5ec16238e2 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:263
    #16 0x7f5ec1bac67c in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:246
    #17 0x7f5ee7310202 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:205
    #18 0x43c240 in __asan::AsanThread::ThreadStart(unsigned long) _asan_rtl_
    #19 0x7f5eea851e99 in start_thread /build/buildd/eglibc-2.15/nptl/pthread_create.c:308
    #20 0x7f5ee99603fc in
Thread T55 (Media Encoder) created by T0 here:
    #0 0x414f7e in __interceptor_pthread_create _asan_rtl_
    #1 0x7f5ee7301924 in _PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:445
    #2 0x7f5ee72ffcfa in PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:528
    #3 0x7f5ec1bb02e7 in nsThread::Init() xpcom/threads/nsThread.cpp:315
    #4 0x7f5ec1bbd6ab in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) xpcom/threads/nsThreadManager.cpp:228
    #5 0x7f5ec1620f84 in NS_NewThread(nsIThread**, nsIRunnable*, unsigned int) xpcom/glue/nsThreadUtils.cpp:67
    #6 0x7f5ec26c7e38 in tag_nsresult NS_NewNamedThread<14ul>(char const (&) [14ul], nsIThread**, nsIRunnable*, unsigned int) objdir-ff-asan-sym/content/media/webrtc/../../../dist/include/nsThreadUtils.h:73
    #7 0x7f5ecf02db8f in mozilla::dom::MediaRecorder::Session::Start() content/media/MediaRecorder.cpp:185:21
    #8 0x7f5ecf02d019 in mozilla::dom::MediaRecorder::Start(mozilla::dom::Optional<int> const&, mozilla::ErrorResult&) content/media/MediaRecorder.cpp:423
    #9 0x7f5ec8d56c70 in mozilla::dom::MediaRecorderBinding::start(JSContext*, JS::Handle<JSObject*>, mozilla::dom::MediaRecorder*, JSJitMethodCallArgs const&) objdir-ff-asan-sym/dom/bindings/./MediaRecorderBinding.cpp:378
    #10 0x7f5ec8d546ae in mozilla::dom::MediaRecorderBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/./MediaRecorderBinding.cpp:538
    #11 0x7f5eddc7c2a7 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:220
    #12 0x7f5eddc7c2a7 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:463
    #13 0x7f5eddad736c in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:513
    #14 0x7f5edbf0fb5c in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/jit/BaselineIC.cpp:8010
    #15 0x7f5eaa3f2b27 in
    #16 0x6110000bee57 in
    #17 0x7f5eaa3eba37 in
==19033==ABORTING

Comment 1

[Ralph Giles (:rillian)]

So much for fixed. cdiehl, which gecko revision is this? We updated the opus code last week.

Comment 2

[Christoph Diehl [:posidron]]

This seems to be fixed on trunk.
Tested with http://hg.mozilla.org/integration/mozilla-inbound/rev/bb06cc9a7088

Comment 3

[Jason Smith [:jsmith]]

(In reply to Christoph Diehl [:cdiehl] from comment #2)
> This seems to be fixed on trunk.
> Tested with
> http://hg.mozilla.org/integration/mozilla-inbound/rev/bb06cc9a7088

Should we close this out then?

Comment 4

[Abhishek Arya]

(In reply to Jason Smith [:jsmith] from comment #3)
> (In reply to Christoph Diehl [:cdiehl] from comment #2)
> > This seems to be fixed on trunk.
> > Tested with
> > http://hg.mozilla.org/integration/mozilla-inbound/rev/bb06cc9a7088
> 
> Should we close this out then?

Yes. I checked on trunk (fix was committed today) and it does not reproduce.

Comment 5

[Ralph Giles (:rillian)]

This required asan to detect the overflow, so adding a regression test isn't worth the hassle, I think.

Comment 6

[u279076]

(In reply to Ralph Giles (:rillian) from comment #5)
> This required asan to detect the overflow, so adding a regression test isn't
> worth the hassle, I think.

Okay, thanks for following up Ralph.

Comment 7

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

Was this trunk only or are older versions affected?

Comment 8

[Paul Adenot (:padenot)]

This needs MediaRecorder + Web Audio, so it's 26 only iirc. I could be wrong, though, not sure when MediaRecorder got preffed-on / landed.

Comment 9

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

Can we suggest a security rating for this issue?

Comment 10

[Paul Adenot (:padenot)]

(In reply to Al Billings [:abillings] from comment #9)
> Can we suggest a security rating for this issue?

imho, it's not sec-sensitive anymore: at the moment, it's just a matter of having NaNs sitting in buffers instead of finite float values. The fix for the NaNs will come later.

Comment 11

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

Can we open the bug?

Comment 12

[Paul Adenot (:padenot)]

I think we can.

Comment 13

[Daniel Veditz [:dveditz]]

Is this a duplicate of bug 927579? If the OPUS upgrade magically fixed this it seems like that might be true. I'm not sure what the "See also" link is supposed to mean.

Comment 14

[Paul Adenot (:padenot)]

It is imho a duplicate, triggered by different test cases.

Comment 15

[Ralph Giles (:rillian)]

What Paul said. Same underlying cause. It looks like it was resolved by the opus update because the libops 1.1 upstream has the patch from bug 927579 comment 14.

The outstanding issue is our code feeding NaNs to the encoder in the first place, but the security issue those values caused in libopus has been fixed. It should be safe to open this bug.