Last Comment Bug 719612 - (CVE-2012-0444) Mozilla Firefox Ogg Vorbis Decoding Memory Corruption (ZDI-CAN-1477)
(CVE-2012-0444)
: Mozilla Firefox Ogg Vorbis Decoding Memory Corruption (ZDI-CAN-1477)
Status: VERIFIED FIXED
[sg:critical][qa!]
: crash, testcase, verified1.9.2
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: mozilla12
Assigned To: Timothy B. Terriberry (:derf)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-19 14:55 PST by Al Billings [:abillings]
Modified: 2016-03-15 14:27 PDT (History)
15 users (show)
kinetik: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
verified
verified
verified
10+
verified
.26+
.26-fixed


Attachments
ZDI POC (password: ZDI-CAN-1477) (2.71 KB, application/java-archive)
2012-01-19 14:56 PST, Al Billings [:abillings]
no flags Details
Proposed fix (4.30 KB, patch)
2012-01-19 17:25 PST, Timothy B. Terriberry (:derf)
kinetik: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑release-
akeybl: approval1.9.2.26+
Details | Diff | Splinter Review

Description Al Billings [:abillings] 2012-01-19 14:55:58 PST
ZDI-CAN-1477:  Mozilla Firefox Ogg Vorbis Decoding Memory Corruption
Remote Code Execution Vulnerability


-- CVSS -----------------------------------------

7.5, AV:N/AC:L/Au:N/C:P/I:P/A:P


-- ABSTRACT -------------------------------------

TippingPoint has identified a vulnerability affecting the following
products:

  Mozilla Firefox


-- VULNERABILITY DETAILS ------------------------

Memory corruption is possible when Firefox is parsing Ogg Vorbis
files. The vulnerability is inside the function static
vorbis_info_floor *floor1_unpack (vorbis_info *vi,oggpack_buffer
*opb), in the file

libvorbis/lib/vorbis_floor1.c:

--- code snip begin ---
	...

  for(j=0,k=0;j<info->partitions;j++){
    count+=info->class_dim[info->partitionclass[j]];
    for(;k<count;k++){
      int t=info->postlist[k+2]=oggpack_read(opb,rangebits);
      if(t<0 || t>=(1<<rangebits))
        goto err_out;
    }
  }
	...
--- code snip end ---

The loop populates the info->postlist array and can be overflowed. The
values which determine the amount to copy are user influenced. The
info->class_dim array is populated with user data, with each element
value being between 1 and 8. The info->partitions value is also taken
from userdata, and can be between 0 and 31. Thus, the total sub-loop
iterations can be 8*31, or 248. Each iteration of the subloop copies
one element (int) into info->postlist, a fixed-size buffer which is
only sized to 65 elements (int). This results in memory corruption and
can be exploited to gain remote code execution.

-- CREDIT ---------------------------------------

This vulnerability was discovered by:

   regenrecht

---
Confirmed crash in trunk from last night with POC at https://crash-stats.mozilla.com/report/index/bp-3f868463-5f01-4da8-966f-b8a642120119. Firefox 9.0.1 crash at https://crash-stats.mozilla.com/report/index/bp-43276710-2344-4ffb-a68c-041232120119.

Password on POC zip is ZDI-CAN-1477.
Comment 2 Al Billings [:abillings] 2012-01-19 15:15:55 PST
This crashes 1.9.2.25 as well with the POC. https://crash-stats.mozilla.com/report/index/bp-57d2d7b6-2731-4e56-910b-c9e6a2120119
Comment 3 Matthew Gregan [:kinetik] 2012-01-19 15:42:19 PST
Adding upstream in to the loop.
Comment 4 Timothy B. Terriberry (:derf) 2012-01-19 17:25:10 PST
Created attachment 590062 [details] [diff] [review]
Proposed fix

This is a simple fix, which errors out if the number of values to be read is larger than the array. I'm not confident this is the correct fix, because rangebits can be as large as 15, leaving far more than 63 legal values for the postlist, and I don't see where in the spec a smaller limit is imposed. Waiting for confirmation from Monty.
Comment 5 Monty Montgomery (:xiphmont) 2012-01-19 22:53:35 PST
> This is a simple fix, which errors out if the number of values to be read is
> larger than the array. I'm not confident this is the correct fix, because
> rangebits can be as large as 15, leaving far more than 63 legal values for
> the postlist

Your conclusion/suggested fix is correct.

What the spec is supposed to say is that there are a maximum number of 65 postlist entries; entry 0 is implicit and set to 0, entry 1 is set to 1<<rangebits, and then up to a maximum of 63 more entries are read in that loop.  The code is supposed to enforce that.

Neither limit is there; the spec is silent and the code does not check.  I'm patching upstream spec and code now.
Comment 6 Monty Montgomery (:xiphmont) 2012-01-19 23:38:43 PST
BTW, let me know when I'm OK to add that test vector to our upstream test vector collection.
Comment 7 Timothy B. Terriberry (:derf) 2012-01-20 00:34:46 PST
Comment on attachment 590062 [details] [diff] [review]
Proposed fix

Okay, with that confirmation, requesting review.
Comment 8 Matthew Gregan [:kinetik] 2012-01-20 00:56:17 PST
Comment on attachment 590062 [details] [diff] [review]
Proposed fix

Stealing review.
Comment 9 Matthew Gregan [:kinetik] 2012-01-20 02:10:12 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/a4d83f2fb3f2
Comment 10 Matthew Gregan [:kinetik] 2012-01-20 02:15:05 PST
Comment on attachment 590062 [details] [diff] [review]
Proposed fix

[Approval Request Comment]
Regression caused by (bug #): not a regression
User impact if declined: potential arbitrary code execution
Testing completed (on m-c, etc.): landed on inbound, passes media tests locally
Risk to taking this patch (and alternatives if risky): very low, may reject some invalid Vorbis files that were previously accepted (if it didn't trash enough memory to cause a crash)
Comment 11 Timothy B. Terriberry (:derf) 2012-01-20 02:30:15 PST
(In reply to Monty from comment #6)
> BTW, let me know when I'm OK to add that test vector to our upstream test
> vector collection.

I don't think there's any reason to wait. Though it wasn't clear to me from the above report if this sample contained an actual exploit or just caused a crash (it didn't actually crash my Linux build, though most likely through dumb luck), you _may_ wish to use a different example that just triggers the out-of-bounds write, and doesn't attempt arbitrary code execution.
Comment 12 Ed Morley [:emorley] 2012-01-21 06:58:56 PST
https://hg.mozilla.org/mozilla-central/rev/a4d83f2fb3f2
Comment 13 Alex Keybl [:akeybl] 2012-01-23 11:17:19 PST
Comment on attachment 590062 [details] [diff] [review]
Proposed fix

[Triage Comment]
Based upon the low-risk nature of this fix, the sg:crit status, and the fact that this is externally known, we've decided to approve. Please land this fix on Aurora, Beta, and 1.9.2 branches. Please specifically land on Beta 10 and the 1.9.2 branches to make our next release in a week.
Comment 14 Al Billings [:abillings] 2012-01-23 12:01:13 PST
I've verified the fix on trunk with the Mac nightly (same machine I verified the PoC on): Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0a1) Gecko/20120123 Firefox/12.0a1
Comment 16 Matthew Gregan [:kinetik] 2012-01-23 15:08:14 PST
(In reply to Matthew Gregan [:kinetik] from comment #15)
> https://hg.mozilla.org/releases/mozilla-1.9.2/rev/952491790dee

For completeness, note that the patch landed here differs in one aspect--the patch hunks that applied to libtremor have been removed because libtremor is not present in 1.9.2.  In all other aspects, the patch is identical to that landed on the other trees.
Comment 17 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-01-24 14:48:09 PST
Verified fixed in Firefox 3.6.26 using the POC.
Comment 18 juan becerra [:juanb] 2012-02-28 15:47:19 PST
Verified on latest Fx13(nightly), Fx11(beta), and Fx12(aurora), and Fx10.0.2. On 9.0.1 the POC crashed the application right away, on the fixed builds there's no crash.
Comment 19 juan becerra [:juanb] 2012-02-28 15:48:26 PST
Comment #18, while using XP.

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