Closed Bug 1079747 Opened 6 years ago Closed 6 years ago
out of bounds read in ns
Media Sniffer::Get MIMEType From Content
Tested on: OS: Ubuntu 12.04 Firefox: ASAN build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/ To reproduce, open the repro-file with ASAN build Firefox ASAn-trace: ==7922==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62d002740400 at pc 0x7f881c0c74c5 bp 0x7fff6c0ba750 sp 0x7fff6c0ba748 READ of size 1 at 0x62d002740400 thread T0 #0 0x7f881c0c74c4 in MatchesMP4 /builds/slave/m-cen-l64-asan-000000000000000/build/toolkit/components/mediasniffer/nsMediaSniffer.cpp:67:0 #1 0x7f881c0c74c4 in nsMediaSniffer::GetMIMETypeFromContent(nsIRequest*, unsigned char const*, unsigned int, nsACString_internal&) /builds/slave/m-cen-l64-asan-000000000000000/build/toolkit/components/mediasniffer/nsMediaSniffer.cpp:143:0 #2 0x7f88161148dd in NS_SniffContent(char const*, nsIRequest*, unsigned char const*, unsigned int, nsACString_internal&) /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/netwerk/base/src/../../../dist/include/nsNetUtil.h:2680:0 #3 0x7f8816233b1b in nsUnknownDecoder::DetermineContentType(nsIRequest*) /builds/slave/m-cen-l64-asan-000000000000000/build/netwerk/streamconv/converters/nsUnknownDecoder.cpp:418:0 #4 0x7f8816232e39 in GetMIMETypeFromContent /builds/slave/m-cen-l64-asan-000000000000000/build/netwerk/streamconv/converters/nsUnknownDecoder.cpp:303:0 #5 0x7f8816232e39 in non-virtual thunk to nsUnknownDecoder::GetMIMETypeFromContent(nsIRequest*, unsigned char const*, unsigned int, nsACString_internal&) /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/netwerk/streamconv/converters/Unified_cpp_converters0.cpp:309:0 #6 0x7f88160ca78e in CallUnknownTypeSniffer(void*, unsigned char const*, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/netwerk/base/src/nsBaseChannel.cpp:725:0 #7 0x7f88160fb955 in CallPeekFunc(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*) /builds/slave/m-cen-l64-asan-000000000000000/build/netwerk/base/src/nsInputStreamPump.cpp:99:0 #8 0x7f8815efd88c in nsPipeInputStream::ReadSegments(tag_nsresult (*)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/io/nsPipe3.cpp:822:0 #9 0x7f88160c9d23 in operator-> /builds/slave/m-cen-l64-asan-000000000000000/build/netwerk/base/src/nsInputStreamPump.cpp:119:0 #10 0x7f88160c9d23 in nsBaseChannel::OnStartRequest(nsIRequest*, nsISupports*) /builds/slave/m-cen-l64-asan-000000000000000/build/netwerk/base/src/nsBaseChannel.cpp:740:0 . . .
http://hg.mozilla.org/mozilla-central/annotate/4bad24a306b2/toolkit/components/mediasniffer/nsMediaSniffer.cpp#l43 'boxSize' is zero so 'boxSize / 4 - 1' wraps around and we loop forever.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Don't read outside of the "ftyp" box. This matches the spec at https://mimesniff.spec.whatwg.org/#signature-for-mp4 much more closely (with the exception of us also looking for "iso[m2]").
Attachment #8502209 - Flags: review?(cpearce)
Comment on attachment 8502209 [details] [diff] [review] bug1079747_v0.patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Unsure, seems hard to do much useful with beyond a DoS. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? The comment on the newly added test describes the bug. Which older supported branches are affected by this flaw? Introduced in Firefox 18. If not all supported branches, which bug introduced the flaw? Bug 567077. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The code hasn't changed much over time, so backporting is trivial. How likely is this patch to cause regressions; how much testing does it need?
Missed a question: (In reply to Matthew Gregan [:kinetik] from comment #3) > How likely is this patch to cause regressions; how much testing does it need? Pretty low, it's a simple change. Worst case is we reject some playing files that we might have attempted to play before.
Attachment #8502209 - Flags: review?(cpearce) → review+
Is this really just a DOS? If so, I'll rate it as sec-low and clear the sec-approval? flag as you can check sec-low bugs into trunk without approval.
I think so. Maybe if you were clever you could use the bug to find a (fixed, by the code) pattern elsewhere in memory, but that'd have to be combined with some other exploit to be useful, I think.
Comment on attachment 8502209 [details] [diff] [review] bug1079747_v0.patch Clearing sec-approval?. As a sec-low, this can just go into trunk. If you'd like it to be taken on branches, please nominate it for those.
Comment on attachment 8502209 [details] [diff] [review] bug1079747_v0.patch Approval Request Comment [Feature/regressing bug #]: Bug 567077, shipped in Firefox 18 [User impact if declined]: DoS, possibly small info leak [Describe test coverage new/current, TBPL]: new test added [Risks and why]: very small, worst case is we'd reject some MP4s we previously accepted [String/UUID change made/needed]: none
Attachment #8502209 - Flags: approval-mozilla-aurora?
Matthew, I think the uplift request for beta is necessary too, right?
Attachment #8502209 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Spoke to bajaj on IRC about this. Given where we are in the v1.4/v2.0 release cycles, we aren't going to take a sec-low without good reason. That said, we'd happily take this as a ride-along fix for v2.1 by virtue of it landing on Beta34 :) Leaving the esr31 decision to the security team, though.
Attachment #8502209 - Flags: approval-mozilla-beta?
Attachment #8502209 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Per comment 13 do you have any thoughts on this for ESR31?
(In reply to Benjamin Kerensa [:bkerensa] from comment #16) > Per comment 13 do you have any thoughts on this for ESR31? As a sec-low, we wouldn't take it unless we had a good reason to think it was especially necessary.
Summary: Heap-buffer-overflow in nsMediaSniffer::GetMIMETypeFromContent → out of bounds read in nsMediaSniffer::GetMIMETypeFromContent
Whiteboard: [adv-main34+] → [adv-main34-]
Reproduced the original issue with the poc from comment #0 using the following build: - http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1412914804/ * fx36 ** https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1416916592/ * fx35 ** https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-linux64-asan/1416904801/ * fx34 ** https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-linux64-asan/1416873749/ Test Cases used: - opened the poc five different times under new window/tabs (prompted to download file, downloaded without issues) - opened the poc five different times under new private window/tabs (prompted to download file, downloaded without issues) - opened the poc five different times under new e10s window/tabs (prompted to download file, downloaded without issues) In total, opened the poc 30 different times on each build without running into the original issue.
You need to log in before you can comment on or make changes to this bug.