Closed Bug 1080986 Opened 9 years ago Closed 9 years ago

out of bounds read in mozilla::WaveReader::LoadListChunk

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla36
Tracking Flags:
Tracking Status
firefox33 --- wontfix
firefox34 --- verified
firefox35 --- verified
firefox36 + verified
firefox-esr31 --- wontfix
b2g-v1.4 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.0M --- wontfix
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: attekett, Assigned: kinetik)

References

Details

(Keywords: crash, sec-low, testcase, Whiteboard: [adv-main34-])

Attachments

(2 files)

repro-file.wav
592 bytes, audio/wav
Details
bug1080986.patch
2.67 KB, patch
rillian
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review
Attached audio repro-file.wav — 
Tested on: 

OS: Ubuntu 12.04

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

Note that the repro-file has to be loaded via separate html-file with the following content:

<html>
<audio autoplay src="repro-file.wav"></audio>
</html>


ASAN-trace:

==21588==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000224fb0 at pc 0x7f60d1d33c64 bp 0x7f609d61bf10 sp 0x7f609d61bf08
READ of size 4 at 0x602000224fb0 thread T45 (Media Decode #1)
    #0 0x7f60d1d33c63 in read<unsigned int> /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/content/media/wave/../../../dist/include/mozilla/Endian.h:603:0   
    #1 0x7f60d1d33c63 in readUint32 /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/content/media/wave/../../../dist/include/mozilla/Endian.h:355:0    
    #2 0x7f60d1d33c63 in ReadUint32BE /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/wave/WaveReader.cpp:74:0    
    #3 0x7f60d1d33c63 in mozilla::WaveReader::LoadListChunk(unsigned int, nsAutoPtr<nsDataHashtable<nsCStringHashKey, nsCString> >&) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/wave/WaveReader.cpp:565:0
    #4 0x7f60d1d30f20 in mozilla::WaveReader::LoadAllChunks(nsAutoPtr<nsDataHashtable<nsCStringHashKey, nsCString> >&) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/wave/WaveReader.cpp:655:0
    #5 0x7f60d1d303d6 in mozilla::WaveReader::ReadMetadata(mozilla::MediaInfo*, nsDataHashtable<nsCStringHashKey, nsCString>**) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/wave/WaveReader.cpp:140:0
    #6 0x7f60d1b7a138 in mozilla::MediaDecoderStateMachine::DecodeMetadata() /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/MediaDecoderStateMachine.cpp:1934:0
    #7 0x7f60d1b77fd2 in mozilla::MediaDecoderStateMachine::CallDecodeMetadata() /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/MediaDecoderStateMachine.cpp:1910:0
    #8 0x7f60d1bc8500 in nsRunnableMethodImpl<void (mozilla::MediaDecoderStateMachine::*)(), void, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/content/media/../../dist/include/nsThreadUtils.h:388:0
.
.
.
0x602000224fb2 is located 0 bytes to the right of 2-byte region [0x602000224fb0,0x602000224fb2)
allocated by thread T45 (Media Decode #1) here:
    #0 0x471d71 in malloc _asan_rtl_:0
    #1 0x7f60d8dfccbd in moz_xmalloc /builds/slave/m-cen-l64-asan-000000000000000/build/memory/mozalloc/mozalloc.cpp:52:0
    #2 0x7f60d1d33068 in operator new[] /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/content/media/wave/../../../dist/include/mozilla/mozalloc.h:220:0    #3 0x7f60d1d33068 in mozilla::WaveReader::LoadListChunk(unsigned int, nsAutoPtr<nsDataHashtable<nsCStringHashKey, nsCString> >&) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/wave/WaveReader.cpp:558:0
    #4 0x7f60d1d30f20 in mozilla::WaveReader::LoadAllChunks(nsAutoPtr<nsDataHashtable<nsCStringHashKey, nsCString> >&) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/wave/WaveReader.cpp:655:0
    #5 0x7f60d1d303d6 in mozilla::WaveReader::ReadMetadata(mozilla::MediaInfo*, nsDataHashtable<nsCStringHashKey, nsCString>**) /builds/slave/m-cen-l64-asan-000000000000000/build/content/media/wave/WaveReader.cpp:140:0
.
.
.
LoadListChunk is called with aChunkSize=2, we allocate a char[aChunkSize], read aChunkSize bytes into it, then call ReadUint32BE on the buffer, which reads 4 bytes from the buffer.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Attached patch bug1080986.patch — — Splinter Review 

        Attachment #8503831 -
        Flags: review?(giles)

    
    

    
  
Comment on attachment 8503831 [details] [diff] [review]
bug1080986.patch

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

I'm not sure you can do much with this, it's a 1-3 byte heap read overrun.  So probably a potential DoS, possibly a small info leak if you're very clever.

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

It's quite obvious from the patch.

Which older supported branches are affected by this flaw?

All.

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

Introduced in bug 778053, which first shipped in Firefox 20.

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

Code hasn't changed much, so patch will probably apply as-is.  Trivial to backport if not.

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

Very simple change, almost no change of regressions.  Worst case would be rejecting files we previously played.

        Attachment #8503831 -
        Flags: sec-approval?

    
  
Blocks: 778053
OS: Linux → All
Hardware: x86_64 → All

    
    

    
  
Comment on attachment 8503831 [details] [diff] [review]
bug1080986.patch

Making this a sec-low rated issue based on comments. As such, it doesn't need sec-approval to go in (only critical and high rated issues do) so I'm clearing the flag. 

If you want this to be fixed on Beta and Aurora in order to ship more quickly, please nominate it for those branches.

        Attachment #8503831 -
        Flags: sec-approval?

        Attachment #8503831 -
        Flags: review?(giles) → review+

    
  
Flags: in-testsuite+

    
    

    
  
Comment on attachment 8503831 [details] [diff] [review]
bug1080986.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 778053, shipped in Firefox 20
[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 rejecting Wave files we used to accept
[String/UUID change made/needed]: none

        Attachment #8503831 -
        Flags: approval-mozilla-aurora?

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/5a6c10740344
Status: ASSIGNED → RESOLVED
Closed: 9 years ago

          status-b2g-v1.4:
          --- → affected

          status-b2g-v2.0:
          --- → affected

          status-b2g-v2.0M:
          --- → affected

          status-b2g-v2.1:
          --- → affected

          status-b2g-v2.2:
          --- → fixed

          status-firefox34:
          unaffected → affected

          status-firefox36:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla36

    
    

    
  
Matthew, same here, any reason why you didn't ask for the beta version?
Flags: needinfo?(kinetik)

    
    

    
  
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.

          status-b2g-v1.4:
          affected → wontfix

          status-b2g-v2.0:
          affected → wontfix

          status-b2g-v2.0M:
          affected → wontfix

    
    

    
  
Comment on attachment 8503831 [details] [diff] [review]
bug1080986.patch

Nope, thanks for checking!
Flags: needinfo?(kinetik)

        Attachment #8503831 -
        Flags: approval-mozilla-beta?

        Attachment #8503831 -
        Flags: approval-mozilla-beta?

        Attachment #8503831 -
        Flags: approval-mozilla-beta+

        Attachment #8503831 -
        Flags: approval-mozilla-aurora?

        Attachment #8503831 -
        Flags: approval-mozilla-aurora+

    
    

    
  
Does this need to land in ESR31?
Flags: needinfo?(abillings)

    
    

    
  
(In reply to Benjamin Kerensa [:bkerensa] from comment #13)
> Does this need to land in ESR31?

No, we don't need to take this on ESR31. It is a minor issue.
Flags: needinfo?(abillings)
Group: core-security

          status-firefox-esr31:
          affected → wontfix
Whiteboard: [adv-main34+]
Alias: CVE-2014-8630
Alias: CVE-2014-8630
Summary: Heap-buffer-overflow in mozilla::WaveReader::LoadListChunk → out of bounds read in mozilla::WaveReader::LoadListChunk
Whiteboard: [adv-main34+] → [adv-main34-]

    
    

    
  
Reproduced the original asan crash using the following build:
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1412913783/

Verified using the following builds:

* fx36:
** https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1416877048/

* fx35
** https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-linux64-asan/1416872492/

* fx34
** https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-linux64-asan/1416873749/

Went through the following test cases:

- opened the poc several times under new windows/tabs
- opened the poc several times under new private windows/tabs
- opened the poc several times under new e10s windows/tabs
Status: RESOLVED → VERIFIED

          status-firefox34:
          fixed → verified

          status-firefox35:
          fixed → verified

          status-firefox36:
          fixed → verified

          You need to log in
          before you can comment on or make changes to this bug.