Bug 1092370 (CVE-2015-0825)

Stack-buffer-underflow in mozilla::MP3FrameParser::ParseBuffer

RESOLVED FIXED in Firefox 36, Firefox OS v2.1

Status

()

--
critical
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: attekett, Assigned: eflores)

Tracking

(4 keywords)

unspecified
mozilla38
x86_64
Linux
crash, csectype-bounds, sec-moderate, testcase
Points:
---
Bug Flags:
sec-bounty -
in-testsuite ?

Firefox Tracking Flags

(firefox34 wontfix, firefox35 wontfix, firefox36+ fixed, firefox37+ fixed, firefox38 fixed, firefox-esr31 unaffected, b2g-v1.4 unaffected, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [adv-main36+][b2g-adv-main2.2+])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8515284 [details]
repro-file.mp3

Tested on:

OS: Ubuntu 14.04

Firefox: ASAN build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1414786499/

ASAN-trace:

==22054==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7fb891d460b9 at pc 0x7fb8c5380eaa bp 0x7fb891d45ea0 sp 0x7fb891d45e98
READ of size 1 at 0x7fb891d460b9 thread T46 (Media Decode #1)
    #0 0x7fb8c5380ea9 in mozilla::MP3FrameParser::ParseBuffer(unsigned char const*, unsigned int, long, unsigned int*) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/media/MP3FrameParser.cpp:336:0
    #1 0x7fb8c53812a7 in mozilla::MP3FrameParser::Parse(char const*, unsigned int, unsigned long) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/media/MP3FrameParser.cpp:470:0
    #2 0x7fb8c5632a08 in mozilla::GStreamerReader::ParseMP3Headers() /builds/slave/m-cen-l64-asan-000000000000000/build/dom/media/gstreamer/GStreamerReader.cpp:285:0
    #3 0x7fb8c5632e63 in mozilla::GStreamerReader::ReadMetadata(mozilla::MediaInfo*, nsDataHashtable<nsCStringHashKey, nsCString>**) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/media/gstreamer/GStreamerReader.cpp:326:0
    #4 0x7fb8c53c98dd in mozilla::MediaDecoderStateMachine::DecodeMetadata() /builds/slave/m-cen-l64-asan-000000000000000/build/dom/media/MediaDecoderStateMachine.cpp:1948:0
    #5 0x7fb8c53c7162 in mozilla::MediaDecoderStateMachine::CallDecodeMetadata() /builds/slave/m-cen-l64-asan-000000000000000/build/dom/media/MediaDecoderStateMachine.cpp:1924:0
    #6 0x7fb8c54254b0 in nsRunnableMethodImpl<void (mozilla::MediaDecoderStateMachine::*)(), void, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/dom/media/../../dist/include/nsThreadUtils.h:388:0
.
.
.
Address 0x7fb891d460b9 is located in stack of thread T46 (Media Decode #1) at offset 25 in frame
    #0 0x7fb8c563283f in mozilla::GStreamerReader::ParseMP3Headers() /builds/slave/m-cen-l64-asan-000000000000000/build/dom/media/gstreamer/GStreamerReader.cpp:272:0

  This frame has 2 object(s):
    [32, 4128) 'bytes' <== Memory access at offset 25 underflows this variable
    [4256, 4260) 'bytesRead'
Thread T46 (Media Decode #1) created by T45 (Media S~hine #1) here:
    #0 0x45e5e5 in __interceptor_pthread_create _asan_rtl_:0
    #1 0x7fb8ce970d8d in _PR_CreateThread /builds/slave/m-cen-l64-asan-000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c:453:0
    #2 0x7fb8ce97090a in PR_CreateThread /builds/slave/m-cen-l64-asan-000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c:544:0
    #3 0x7fb8c15134db in nsThread::Init() /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/threads/nsThread.cpp:455:0
    #4 0x7fb8c1518d3c in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/threads/nsThreadManager.cpp:269:0
    #5 0x7fb8c151a298 in nsThreadPool::PutEvent(nsIRunnable*) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/threads/nsThreadPool.cpp:101:0
    #6 0x7fb8c151b9d9 in nsThreadPool::Dispatch(nsIRunnable*, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/threads/nsThreadPool.cpp:261:0
    #7 0x7fb8c5418cbb in mozilla::MediaTaskQueue::DispatchLocked(mozilla::TemporaryRef<nsIRunnable>, mozilla::MediaTaskQueue::DispatchMode) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/media/MediaTaskQueue.cpp:53:0
Severity: normal → critical
Keywords: crash, csectype-bounds, sec-high, testcase
Flags: sec-bounty?
Any ideas as to a potential dev owner for this?
Flags: needinfo?(cdiehl)
Flags: needinfo?(cdiehl) → needinfo?(cpearce)
Flags: needinfo?(cpearce) → needinfo?(edwin)
Group: media-core-security
Created attachment 8525037 [details] [diff] [review]
bug1092370.patch
Assignee: nobody → edwin
Status: NEW → ASSIGNED
Flags: needinfo?(edwin)
Attachment #8525037 - Flags: review?(cpearce)
Comment on attachment 8525037 [details] [diff] [review]
bug1092370.patch

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

::: dom/media/MP3FrameParser.cpp
@@ +337,5 @@
>          // Found an ID3 header. We don't care about the body of the header, so
>          // just skip past.
>          buffer = ch + mID3Parser.GetHeaderLength() - (ID3_HEADER_LENGTH - 1);
> +
> +        if (buffer <= ch) {

Maybe instead:

  if (buffer <= ch || buffer >= aBufferEnd) {
    return NS_ERROR_FAILURE;
  }

?
Attachment #8525037 - Flags: review?(cpearce) → review+
Keywords: checkin-needed
Needs sec-approval since it's a sec-high and it's unclear what branches are affected.
https://wiki.mozilla.org/Security/Bug_Approval_Process
Flags: needinfo?(edwin)
Keywords: checkin-needed, sec-high
Created attachment 8544917 [details] [diff] [review]
bug1092370.patch

addressed review comment. carry over r+
Attachment #8525037 - Attachment is obsolete: true
Attachment #8544917 - Flags: review+
Comment on attachment 8544917 [details] [diff] [review]
bug1092370.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Could be straightforward to craft an MP3 that will segfault Firefox. Very little, if any, risk of much else.

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

Which older supported branches are affected by this flaw?
Release and up.

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

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

How likely is this patch to cause regressions; how much testing does it need?
Not likely.
Flags: needinfo?(edwin)
Attachment #8544917 - Flags: sec-approval?
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → unaffected
status-b2g-v2.0M: --- → unaffected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → affected
status-firefox34: --- → wontfix
status-firefox35: --- → wontfix
status-firefox36: --- → affected
status-firefox37: --- → affected
status-firefox-esr31: --- → unaffected
Comment on attachment 8544917 [details] [diff] [review]
bug1092370.patch

sec-approval+ for trunk checkin on January 26. At that point, we should take it on the (then) Aurora and Beta branches as well if you nominate patches there.
Attachment #8544917 - Flags: sec-approval? → sec-approval+
tracking-firefox36: --- → +
tracking-firefox37: --- → +
Whiteboard: [checkin on 1/26]
Keywords: sec-high
The sec-high keyword got accidentally removed, so I'm just adding it back.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e50bc3e146bc

Please request Aurora/Beta approval on this when you get a chance.
Flags: needinfo?(edwin)
Flags: in-testsuite?
Whiteboard: [checkin on 1/26]
Comment on attachment 8544917 [details] [diff] [review]
bug1092370.patch

Oops. Caused by blinding following review comment. |buffer| is meant to be allowed to overflow.
Attachment #8544917 - Attachment is obsolete: true
Flags: needinfo?(edwin)
Comment on attachment 8525037 [details] [diff] [review]
bug1092370.patch

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

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

Which older supported branches are affected by this flaw?

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

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

How likely is this patch to cause regressions; how much testing does it need?
Attachment #8525037 - Attachment is obsolete: false
Attachment #8525037 - Flags: sec-approval?
Er, refer to sec-approval in comment 6.
Attachment #8525037 - Flags: sec-approval? → sec-approval+
Comment on attachment 8525037 [details] [diff] [review]
bug1092370.patch

Approval Request Comment
[Feature/regressing bug #]: MP3 playback
[User impact if declined]: could crash on particularly bad MP3s
[Describe test coverage new/current, TreeHerder]: None
[Risks and why]: None
[String/UUID change made/needed]: None
Attachment #8525037 - Flags: approval-mozilla-beta?
Attachment #8525037 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a1961f96d4a7
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-b2g-v2.1S: --- → affected
status-b2g-master: --- → fixed
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Attachment #8525037 - Flags: approval-mozilla-beta?
Attachment #8525037 - Flags: approval-mozilla-beta+
Attachment #8525037 - Flags: approval-mozilla-aurora?
Attachment #8525037 - Flags: approval-mozilla-aurora+
Group: media-core-security
(In reply to Edwin Flores [:eflores] [:edwin] from comment #6)
> [Security approval request comment]
> How easily could an exploit be constructed based on the patch?
> Could be straightforward to craft an MP3 that will segfault Firefox. Very
> little, if any, risk of much else.

What if Firefox does NOT segfault, because an exploit manipulates memory to put this buffer near other buffers owned by Firefox? Would that random data be read as part of an MP3? Would that MP3 data be accessible to the page afterwards?
Flags: needinfo?(edwin)
(In reply to Daniel Veditz [:dveditz] from comment #19)
> What if Firefox does NOT segfault, because an exploit manipulates memory to
> put this buffer near other buffers owned by Firefox? Would that random data
> be read as part of an MP3? Would that MP3 data be accessible to the page
> afterwards?

This patch causes us to bail immediately in the event of an underrun. Overruns are expected and handled.
Flags: needinfo?(edwin)
I meant what happened _before_ the patch? If memory were arranged in a useful way so it didn't just get a simple access violation what is the code going to do with the bogus memory it accesses? Is it being read or written to? If it's a read is it just stream data, or is it reading objects that might be interpreted as pointers?
Flags: needinfo?(edwin)
(In reply to Daniel Veditz [:dveditz] from comment #21)
> I meant what happened _before_ the patch? If memory were arranged in a
> useful way so it didn't just get a simple access violation what is the code
> going to do with the bogus memory it accesses? Is it being read or written
> to? If it's a read is it just stream data, or is it reading objects that
> might be interpreted as pointers?

Ah, right.

The data here is merely being read and parsed; no writing; no interpreting as pointers. In the unlikely event that the buffer pointer ends up on something that looks like MP3, there's still very (very) little risk of meaningful information leakage.
Flags: needinfo?(edwin)
Whiteboard: [adv-main36+]
Alias: CVE-2015-0825
> In the unlikely event that the buffer pointer ends up on something that looks like MP3,
> there's still very (very) little risk of meaningful information leakage.

Assuming it does get past the MP3 parser, is there any way to get the data back into the page? Or does it just get sent off to media players? I think the WebAudio interface can probably get data, right?
Flags: needinfo?(edwin)
(In reply to Daniel Veditz [:dveditz] from comment #23)
> Assuming it does get past the MP3 parser, is there any way to get the data
> back into the page? Or does it just get sent off to media players? I think
> the WebAudio interface can probably get data, right?

Not really, no. The parser here is only used to estimate the duration of a given MP3.

We can only rewind here by 9 bytes. There are two cases for the parser to keep going:

Case 1: It looks like another ID3 header. This doesn't matter too much. The length field will overlap with the original ID3 header, which will make our pointer positive again. Moreover, we don't expose any of the ID3 data from this parser.

Case 2: It looks like an MP3 frame. Even in this case, we don't expose much data. We only parse the MP3 frame for duration estimation. Whatever *does* leak out in the form of a duration will have gone through a series of lookup tables and arithmetic operations [1].

Each MP3 frame header is about 3 bytes long. If we suppose we can pack just the headers into the 9 bytes available, we end up with 3 headers. Naively combining the tables, we can get 1000 different values, or about 10 bits per frame of information leakage if we assume an even distribution.

This theoretically leaves us with an upper bound of 30 bits of information leaked. In practice, a zero-length frame body implies a fairly trivial header so we'd really only fit one or two frames (even two is pushing the bounds of usefulness).

If somebody were to somehow use 10 to 20 bits of munged up information from somewhere in the 9 bytes preceding our buffer against us, I would be very, very impressed.

[1] - https://dxr.mozilla.org/mozilla-central/source/dom/media/MP3FrameParser.cpp#76
Flags: needinfo?(edwin)
None of those cases look highly exploitable
Flags: sec-bounty? → sec-bounty-
Keywords: sec-high → sec-moderate
Whiteboard: [adv-main36+] → [adv-main36+][b2g-adv-main2.2+]

Updated

3 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.