Bug 1175396 (CVE-2015-4475)

out of bounds read at mozilla::AudioSink::PlayFromAudioQueue()

VERIFIED FIXED in Firefox 40

Status

()

defect
--
critical
VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: aki.helin, Assigned: kinetik)

Tracking

({csectype-bounds, csectype-uaf, sec-high})

Trunk
mozilla42
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox39 wontfix, firefox40+ verified, firefox41+ fixed, firefox42+ verified, firefox-esr31 wontfix, firefox-esr3840+ verified, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 fixed, b2g-v2.2r fixed, b2g-master fixed)

Details

(Whiteboard: [adv-main40+][adv-esr38.2+])

Attachments

(4 attachments, 1 obsolete attachment)

Reporter

Description

4 years ago
A use-after-free occurs whenever the attached mp3 is opened via the page I'll add in a sec. This happens at least in freshly installed 64-bit Ubuntu using the last few tinderbox asan builds.

Bug 1093150 is about something on mostly Windows with a very similar trace.


==9814==ERROR: AddressSanitizer: heap-use-after-free on address 0x6210001668ff at pc 0x7f803058eb2b bp 0x7f80017a21c0 sp 0x7f80017a21b8
READ of size 9216 at 0x6210001668ff thread T36 (Media Audio)
    #0 0x7f803058eb2a in mozilla::AudioStream::Write(float const*, unsigned int, mozilla::TimeStamp*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/media/AudioStream.h:117
    #1 0x7f803058b7a8 in mozilla::AudioSink::PlayFromAudioQueue() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/media/AudioSink.cpp:354
    #2 0x7f803058941c in mozilla::AudioSink::AudioLoop() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/media/AudioSink.cpp:219
    #3 0x7f80305a9860 in nsRunnableMethodImpl<void (mozilla::AudioSink::*)(), true>::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/obj-firefox/dom/media/../../dist/include/nsThreadUtils.h:618
    #4 0x7f802c06dbe7 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThread.cpp:846
    #5 0x7f802c0e78aa in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #6 0x7f802c93ccd9 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/glue/MessagePump.cpp:326
    #7 0x7f802c8ca4ec in MessageLoop::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #8 0x7f802c06a4fc in nsThread::ThreadFunc(void*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThread.cpp:359
    #9 0x7f8038c51135 in _pt_root /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:212
    #10 0x7f803928e6a9 in start_thread /build/buildd/glibc-2.21/nptl/pthread_create.c:333
    #11 0x7f8029c03eec in clone /build/buildd/glibc-2.21/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109
Reporter

Comment 1

4 years ago
Keywords: sec-high
Summary: heap-use-after-free at mozilla::AudioSink::PlayFromAudioQueue() → out of bounds read at mozilla::AudioSink::PlayFromAudioQueue()
Assignee

Updated

4 years ago
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Assignee

Comment 2

4 years ago
Posted patch v0Splinter Review
This (corrupt/invalid?) MP3 changes sample format multiple times.  The AudioStream assumes the audio buffers written to it are in the format specified via AudioStream::Init and uses these format details to convert buffer lengths from frames to bytes.  In this particular case, we initialize AudioStream for 44100Hz/2 channels and later pass it an AudioData buffer containing 44100Hz/1 channel data and (depending on memory allocation size/alignment) read off the end of the buffer or fault in memcpy.
Attachment #8628086 - Flags: review?(jwwang)
Assignee

Updated

4 years ago
Blocks: 1093150
Comment on attachment 8628086 [details] [diff] [review]
v0

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

::: dom/media/AudioSink.cpp
@@ +354,5 @@
> +  if (audio->mRate == mInfo.mRate && audio->mChannels == mInfo.mChannels) {
> +    mAudioStream->Write(audio->mAudioData, audio->mFrames);
> +  } else {
> +    SINK_LOG_V("mismatched sample format mInfo=[%uHz/%u channels] audio=[%uHz/%u channels]",
> +               mInfo.mRate, mInfo.mChannels, audio->mRate, audio->mChannels);

Mismatch looks like a warning to me.
Attachment #8628086 - Flags: review?(jwwang) → review+
Assignee

Comment 4

4 years ago
Comment on attachment 8628086 [details] [diff] [review]
v0

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

Seems difficult, triggering the bug causes either a fault (unusual, depends on memory layout) or writing out-of-bounds data to the system audio device.  The latter case happens below any scriptable audio capture level exists, so it'd be very difficult for an attacker to get access to the OOB memory contents.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Patch is trivial, so fairly obvious.

Which older supported branches are affected by this flaw?

All; bug has existed ~forever and has been exposed at least since MP3 support was added.

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

Existing patch should apply with minimal fuss.

How likely is this patch to cause regressions; how much testing does it need?

Very low.
Attachment #8628086 - Flags: sec-approval?
This has sec-approval+ to go in but not until two weeks into the new cycle, which is July 14. We'll want this on Beta and Aurora as well so please nominate for that (or create appropriate patches and nominate them).
Whiteboard: [checkin on 7/14]
Attachment #8628086 - Flags: sec-approval? → sec-approval+
Assignee

Comment 6

4 years ago
Comment on attachment 8628086 [details] [diff] [review]
v0

Patch applies verbatim to aurora and beta.
Attachment #8628086 - Flags: approval-mozilla-beta?
Attachment #8628086 - Flags: approval-mozilla-aurora?
Comment on attachment 8628086 [details] [diff] [review]
v0

Approval for branches after it goes onto trunk.
Attachment #8628086 - Flags: approval-mozilla-beta?
Attachment #8628086 - Flags: approval-mozilla-beta+
Attachment #8628086 - Flags: approval-mozilla-aurora?
Attachment #8628086 - Flags: approval-mozilla-aurora+
Can you please nominate this for esr38 as well?
Assignee

Updated

4 years ago
Flags: needinfo?(kinetik)
Attachment #8628086 - Flags: approval-mozilla-esr38?
https://hg.mozilla.org/mozilla-central/rev/25bb4971646c
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Assignee

Updated

4 years ago
Duplicate of this bug: 1093150
Comment on attachment 8628086 [details] [diff] [review]
v0

sec high, taking it.
Attachment #8628086 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Assignee

Comment 17

4 years ago
Posted patch v0 for b2g32 (obsolete) — Splinter Review
Flags: needinfo?(kinetik)
Assignee

Comment 19

4 years ago
Posted patch v1 for b2g32Splinter Review
v0 b2g fails to build because AudioData doesn't carry the sample rate (it was added in 34).  Only the channel check is critical for this fix, so here's a simplified fix for b2g32.
Attachment #8635007 - Attachment is obsolete: true
Flags: qe-verify+
Flags: sec-bounty?
Encountered the same result as in comment 0 with nightly asan build from 2015-06-16, under Ubuntu 13.10 64-bit, including a tab crash.
Verified fixed with 40.0b6 (Build ID: 20150720220238), ESR 38.1.0esrpre (Build ID: 20150722183818) and latest 42.0a1 asan build.
Flags: sec-bounty? → sec-bounty+
Flags: qe-verify+
Whiteboard: [adv-main40+][adv-esr38.2+]
Alias: CVE-2015-4475

Updated

4 years ago
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.