785967 - (CVE-2012-4186) Heap-buffer-overflow in nsWaveReader::DecodeAudioData

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Atte Kettunen]

Attachment: Repro-file

This bug was filed from the Socorro interface and is 
report bp-edaabcc3-a4c5-4935-b00e-940a22120827 .
============================================================= 

I'm trying to minimize the testcase further and I'll add new testcase if I make any progress worth it.

ASAN-report from opt-build:

==10566== ERROR: AddressSanitizer heap-buffer-overflow on address 0x7f8860191081 at pc 0x7f8888de9ee1 bp 0x7f88311df4e0 sp 0x7f88311df4d8
READ of size 1 at 0x7f8860191081 thread T18
    #0 0x7f8888de9ee1 in nsWaveReader::DecodeAudioData() /home/attekett/firefox/src/content/media/wave/nsWaveReader.cpp:192
0x7f8860191081 is located 1 bytes to the right of 4096-byte region [0x7f8860190080,0x7f8860191080)
allocated by thread T18 here:
    #0 0x425141 in __interceptor_malloc ??:0
    #1 0x7f888d468228 in moz_xmalloc /home/attekett/firefox/src/memory/mozalloc/mozalloc.cpp:57
Thread T18 created by T17 here:
    #0 0x420e15 in pthread_create ??:0
    #1 0x7f888efac45f in _PR_CreateThread /home/attekett/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:393
    #2 0x7f888efabeb8 in PR_CreateThread /home/attekett/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:476
==10566== ABORTING
Stats: 329M malloced (591M for red zones) by 1746645 calls
Stats: 58M realloced by 20510 calls
Stats: 256M freed by 217843 calls
Stats: 97M really freed by 154806 calls
Stats: 864M (221280 full pages) mmaped in 209 calls
  mmaps   by size class: 8:1589151; 9:32764; 10:16380; 11:16376; 12:3072; 13:1536; 14:1280; 15:256; 16:384; 17:1248; 18:176; 19:40; 20:16; 21:2; 22:12; 23:4; 24:1;
  mallocs by size class: 8:1664564; 9:38223; 10:17175; 11:18265; 12:2582; 13:1803; 14:1539; 15:361; 16:558; 17:1303; 18:195; 19:41; 20:17; 21:2; 22:12; 23:4; 24:1;
  frees   by size class: 8:153178; 9:28974; 10:13767; 11:14943; 12:1700; 13:1601; 14:1361; 15:309; 16:484; 17:1289; 18:169; 19:40; 20:15; 21:2; 22:11;
  rfrees  by size class: 8:114243; 9:17260; 10:9448; 11:10197; 12:972; 13:748; 14:839; 15:190; 16:184; 17:695; 18:25; 19:4; 20:1;
Stats: malloc large: 1575 small slow: 4633
Shadow byte and word:
  0x1ff10c032210: fa
  0x1ff10c032210: fa fa fa fa fa fa fa fa
More shadow bytes:
  0x1ff10c0321f0: 00 00 00 00 00 00 00 00
  0x1ff10c0321f8: 00 00 00 00 00 00 00 00
  0x1ff10c032200: 00 00 00 00 00 00 00 00
  0x1ff10c032208: 00 00 00 00 00 00 00 00
=>0x1ff10c032210: fa fa fa fa fa fa fa fa
  0x1ff10c032218: fa fa fa fa fa fa fa fa
  0x1ff10c032220: fa fa fa fa fa fa fa fa
  0x1ff10c032228: fa fa fa fa fa fa fa fa
  0x1ff10c032230: fa fa fa fa fa fa fa fa

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: patch v0

Comment 2

[Matthew Gregan [:kinetik]]

In a local ASAN build, without the patch, I get the same failure as comment 0.  With patch applied, the file is rejected as expected:
WARNING: Invalid WAVE metadata: file /home/kinetik/mozilla-asan/content/media/wave/nsWaveReader.cpp, line 445

https://hg.mozilla.org/mozilla-central/rev/0eebcc79ee05

Comment 3

[Matthew Gregan [:kinetik]]

Comment on attachment 655899 [details] [diff] [review]
patch v0

[Approval Request Comment]
Regression caused by (bug #): forever (since this coded was added)
User impact if declined: likely exploitable heap overwrite
Testing completed (on m-c, etc.): tested locally in normal and ASAN builds
Risk to taking this patch (and alternatives if risky): extremely low, will reject some invalid files that may previously have played corrupt audio

Do we also want this on ESR10?

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

We'll need a sec rating on this to evaluate for ESR and in case there are regressions or this needs to be backed out.  Why is this requesting mozilla-release approval?  You say this bug has been here "forever" since this code was added - what release was that?

Comment 5

[Matthew Gregan [:kinetik]]

(In reply to Lukas Blakk [:lsblakk] from comment #4)
> We'll need a sec rating on this to evaluate for ESR and in case there are
> regressions or this needs to be backed out.

Does the security team do that evaluation or me?  I couldn't find any docs in the wiki that explained the process, and the bugs I've worked on in the past had already been tagged.

> Why is this requesting mozilla-release approval?  You say this bug has been here
> "forever" since this code was added - what release was that?

Sorry, it was Firefox 3.5.  But it turns out I was wrong--the original code that shipped in 3.5 didn't validate the frame size, but it also didn't use that value to size buffers.  So this became exploitable when the code was refactored in bug 635649, which shipped in Firefox 5.

https://hg.mozilla.org/releases/mozilla-aurora/rev/e101e4c1dcf8
https://hg.mozilla.org/releases/mozilla-beta/rev/8fbe6ae6f69a

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 655899 [details] [diff] [review]
patch v0



Not a chemspill issue so minusing for mozilla-release but we'll take this on the ESR.

Comment 7

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/a0f5a71b35b7

Comment 9

[Daniel Veditz [:dveditz]]

Bounty awarded $3000

Comment 10

[u279076]

Attached testcase verified reproducible with parent to the resolved changeset in comment 2.

Verified fixed with:
 * Firefox 18.0a1 Nightly debug asan 2012-09-26
 * Firefox 17.0a2 Aurora debug asan 2012-09-26

Unverifiable for 16.0 Beta and ESR10 due to lack of builds. Marking qa- until these builds exist.