Bug 1200148 (CVE-2015-4511)

Heap-buffer-overflow due to overflow in nestegg_track_codec_data

VERIFIED FIXED in Firefox 41

Status

()

defect
VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: attekett, Assigned: kinetik)

Tracking

(4 keywords)

unspecified
mozilla43
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox41+ verified, firefox42+ verified, firefox43+ verified, firefox-esr3841+ verified, b2g-v2.0 wontfix, b2g-v2.0M wontfix, b2g-v2.1 wontfix, b2g-v2.1S wontfix, b2g-v2.2 affected, b2g-v2.2r affected, b2g-master fixed)

Details

(Whiteboard: [adv-main41+][adv-esr38.3+])

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

4 years ago
Posted video repro-file.webm
Tested on: 

OS: Ubuntu 14.04

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

ASAN-trace:

=================================================================
==28309==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6070000d1b9b at pc 0x46fac7 bp 0x7f81856f75f0 sp 0x7f81856f6db0
READ of size 110 at 0x6070000d1b9b thread T26 (MediaPl~back #1)
    #0 0x46fac6 in memcpy /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:366
    #1 0x7f81acac2b87 in void AssignRangeAlgorithm<true, true>::implementation<unsigned char, unsigned char, unsigned long, unsigned long>(unsigned char*, unsigned long, unsigned long, unsigned char const*
) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/media/libstagefright/../../dist/include/nsTArray.h:559
    #2 0x7f81aca86265 in unsigned char* nsTArray_Impl<unsigned char, nsTArrayInfallibleAllocator>::AppendElements<unsigned char, nsTArrayInfallibleAllocator>(unsigned char const*, unsigned long) /builds/sl
ave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/media/libstagefright/../../dist/include/nsTArray.h:1510
    #3 0x7f81b07467d0 in mozilla::XiphHeadersToExtradata(mozilla::MediaByteBuffer*, nsTArray<unsigned char const*> const&, nsTArray<unsigned long> const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build
/src/dom/media/XiphExtradata.cpp:26
    #4 0x7f81b098652b in mozilla::WebMDemuxer::ReadMetadata() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/media/webm/WebMDemuxer.cpp:412
    #5 0x7f81b09857c6 in mozilla::WebMDemuxer::Init() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/media/webm/WebMDemuxer.cpp:161
    #6 0x7f81b062c2e2 in mozilla::MediaFormatReader::AsyncReadMetadata() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/media/MediaFormatReader.cpp:293
    #7 0x7f81b06a8815 in nsRefPtr<mozilla::MozPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true> > mozilla::detail::MethodCallInvokeHelper<nsRefPtr<mozilla::MozPromise<nsR
efPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true> >, mozilla::MediaDecoderReader>(nsRefPtr<mozilla::MozPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason,
 true> > (mozilla::MediaDecoderReader::*)(), mozilla::MediaDecoderReader*, mozilla::Tuple<>&, mozilla::IndexSequence<>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/media/../../di
st/include/mozilla/MozPromise.h:869
    #8 0x7f81b06a86ce in mozilla::detail::MethodCall<mozilla::MozPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true>, mozilla::MediaDecoderReader>::Invoke() /builds/slave/m
-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/media/../../dist/include/mozilla/MozPromise.h:894
    #9 0x7f81b06a849c in mozilla::detail::ProxyRunnable<mozilla::MozPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true>, mozilla::MediaDecoderReader>::Run() /builds/slave/m
-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/media/../../dist/include/mozilla/MozPromise.h:912
    #10 0x7f81acc68727 in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/xpcom/threads/../../dist/include/mozilla/TaskDispatcher.h:
180
.
.
.
0x6070000d1b9b is located 0 bytes to the right of 75-byte region [0x6070000d1b50,0x6070000d1b9b)
allocated by thread T26 (MediaPl~back #1) here:
    #0 0x48290b in realloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:95
    #1 0x7f81accafb02 in mozilla::CountingAllocatorBase<NesteggReporter>::CountingRealloc(void*, unsigned long) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/xpcom/build/../../dist/include/mozilla/CountingAllocatorBase.h:79
    #2 0x7f81b2b11113 in halloc /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/media/libnestegg/src/halloc.c:78
    #3 0x7f81b2b197c4 in ne_pool_alloc /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/media/libnestegg/src/nestegg.c:534
    #4 0x7f81b2b1970d in ne_read_binary /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/media/libnestegg/src/nestegg.c:753
    #5 0x7f81b2b191da in ne_read_simple /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/media/libnestegg/src/nestegg.c:1017
    #6 0x7f81b2b128cf in ne_parse /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/media/libnestegg/src/nestegg.c:1088
    #7 0x7f81b2b11c23 in nestegg_init /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/media/libnestegg/src/nestegg.c:1948
    #8 0x7f81b0985bc0 in mozilla::WebMDemuxer::ReadMetadata() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/media/webm/WebMDemuxer.cpp:272
    #9 0x7f81b09857c6 in mozilla::WebMDemuxer::Init() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/media/webm/WebMDemuxer.cpp:161
    #10 0x7f81b062c2e2 in mozilla::MediaFormatReader::AsyncReadMetadata() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/media/MediaFormatReader.cpp:293
.
.
.
Is this a stagefright bug or a webm bug?
Keywords: crash, sec-high, testcase
WebM(In reply to Daniel Veditz [:dveditz] from comment #1)
> Is this a stagefright bug or a webm bug?

WebM.
I believe this is an overflow in nestegg_track_codec_data(). It does not check that the sizes of the headers it parses out of the extra data are actually valid.

The amount of data in the codec_private field in the test file is 43 bytes. It claims there are two headers, and the first claims to be 110 bytes. It computes the size of the second as 18446744073709551547 bytes.

Then we try to pack them to send to the decoder and we crash.
Assignee

Comment 4

4 years ago
Discussed this with Tim offline.  I'll adopt the parsing code in XiphExtradataToHeaders for nestegg_track_codec_data.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED

Updated

4 years ago
Group: core-security → media-core-security
Assignee

Comment 5

4 years ago
Posted patch bug1200148_v0.patch (obsolete) — Splinter Review
Attachment #8657681 - Flags: review?(tterribe)
Comment on attachment 8657681 [details] [diff] [review]
bug1200148_v0.patch

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

r=me with a few comments (address or not as you see fit).

::: src/nestegg.c
@@ +2286,5 @@
> +        size += *p;
> +        avail -= 1;
> +        if (*p++ != 255)
> +          break;
> +      }

I know this is copied from my code, but looking at it I don't know why I didn't just make this a do-while loop.

@@ +2295,5 @@
>      }
> +    sizes[i] = avail - total;
> +
> +    if (item >= count)
> +      return -1;

Any reason not to this check before the loop above?
Attachment #8657681 - Flags: review?(tterribe) → review+
Assignee

Comment 7

4 years ago
(In reply to Timothy B. Terriberry (:derf) from comment #6)
> I know this is copied from my code, but looking at it I don't know why I
> didn't just make this a do-while loop.

Seems nicer, I've changed the nestegg version.

> @@ +2295,5 @@
> Any reason not to this check before the loop above?

No.  I moved it up and merged it with the count > 3 check.
Assignee

Comment 8

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

Medium.

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

Pretty much, and the patch itself is small enough that it's pretty obvious where to explore to produce an exploit.

Which older supported branches are affected by this flaw?

All, although the severity is less/different on older branches due to the simpler way this API was used.

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

N/A

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

Patch should apply as-is, or be trivial to backport at worst.

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

Patch is pretty safe, the same approach has been used elsewhere (in XiphExtradataToHeaders) for a couple of weeks, existing regression test suite passes locally.
Attachment #8657681 - Attachment is obsolete: true
Attachment #8657965 - Flags: sec-approval?
Attachment #8657965 - Flags: review+
Flags: sec-bounty?
Sec-approval+ for trunk. We should get patches made and nominated for Aurora, Beta, and ESR38 once it is on trunk.
Attachment #8657965 - Flags: sec-approval? → sec-approval+
XiphHeadersToExtradata is only in 43 and was introduced in bug 1196353
Assignee

Comment 11

4 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #10)
> XiphHeadersToExtradata is only in 43 and was introduced in bug 1196353

Read comment 3.
Summary: Heap-buffer-overflow in mozilla::XiphHeadersToExtradata → Heap-buffer-overflow due to overflow in nestegg_track_codec_data
Matthew, please nominate beta patch for uplift? We gtb Beta9 tomorrow so it would be nice if we have the patch ready to land asap.
Flags: needinfo?(kinetik)
also nom it for ESR please :) Thanks.
Assignee

Comment 14

4 years ago
Comment on attachment 8657965 [details] [diff] [review]
bug1200148_v1.patch

The patch applies to ESR38 as-is, except for the update to README_MOZILLA, but that can simply be ignored as it's only updated the upstream git commit ID which doesn't make sense when backporting a patch.

Approval Request Comment
[Feature/regressing bug #]: bug 566246 (when libnestegg as added), but may not have become exploitable until recently due to change in API usage pattern
[User impact if declined]: possible memory corruption, crash, and/or remotely exploitable bug
[Describe test coverage new/current, TreeHerder]: n/a (regression test added to upstream repo)
[Risks and why]: 
[String/UUID change made/needed]: none
Flags: needinfo?(kinetik)
Attachment #8657965 - Flags: approval-mozilla-esr38?
Attachment #8657965 - Flags: approval-mozilla-beta?
Attachment #8657965 - Flags: approval-mozilla-b2g37?
Attachment #8657965 - Flags: approval-mozilla-aurora?
Assignee

Updated

4 years ago
Keywords: checkin-needed
Comment on attachment 8657965 [details] [diff] [review]
bug1200148_v1.patch

Taking it as it's a sec-high. Beta41+, Aurora42+
Attachment #8657965 - Flags: approval-mozilla-beta?
Attachment #8657965 - Flags: approval-mozilla-beta+
Attachment #8657965 - Flags: approval-mozilla-aurora?
Attachment #8657965 - Flags: approval-mozilla-aurora+
KWierso told me that this bug hasn't landed on inbound so this will miss Beta9.
Landed to beta in https://hg.mozilla.org/releases/mozilla-beta/rev/360c43a9bb74

Still need to land to inbound and merge that to m-c and also uplift to aurora, but those are on less of a time crunch.
Flags: qe-verify+
https://hg.mozilla.org/mozilla-central/rev/78050272add4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Group: media-core-security → core-security-release
Comment on attachment 8657965 [details] [diff] [review]
bug1200148_v1.patch

Approved for uplift to esr38. Stability improvement, has tests.
Attachment #8657965 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Blocks: 566246, 1196353
Flags: sec-bounty? → sec-bounty+
Keywords: csectype-bounds
Comment on attachment 8657965 [details] [diff] [review]
bug1200148_v1.patch

sec-high/crit don't need approval to land on affected B2G branches once the fix has landed on affected Fx branches.
Attachment #8657965 - Flags: approval-mozilla-b2g37?
Whiteboard: [adv-main41+][adv-esr38.3+]
Alias: CVE-2015-4511
Reproduced the initial crash and similar asan-trace using old Asan debug build from 2015-08-31, verified that the crash does not occur using latest Nightly and Aurora asan debug build from 2015-09-21 (Ubuntu 14.04 64-bit), Firefox ESR 38.3.0 build 2 and Firefox 41.0 build 3 across platforms (Windows 10 32-bit, Ubuntu 14.04 64-bit and Mac OS X 10.10.5).
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.