As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact
Last Comment Bug 675747 - (CVE-2011-3005) ogg crash [@ nsOggReader::ReadHeaders]
: ogg crash [@ nsOggReader::ReadHeaders]
: crash, regression, testcase, verified-aurora, verified-beta
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 5 Branch
: All All
: -- critical (vote)
: mozilla8
Assigned To: Chris Pearce (:cpearce)
: Maire Reavy [:mreavy] Please needinfo me
Depends on:
  Show dependency treegraph
Reported: 2011-08-01 14:41 PDT by sczimmer
Modified: 2015-05-07 15:20 PDT (History)
8 users (show)
rforbes: sec‑bounty+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

example.ogg (1.82 MB, video/ogg)
2011-08-01 14:41 PDT, sczimmer
no flags Details
stack (6.78 KB, text/plain)
2011-08-01 15:07 PDT, Mats Palmgren (:mats)
no flags Details
Patch v1 (1.91 KB, patch)
2011-08-01 16:30 PDT, Chris Pearce (:cpearce)
cajbir.bugzilla: review-
Details | Diff | Splinter Review
Patch v2 (43.33 KB, patch)
2011-08-03 14:18 PDT, Chris Pearce (:cpearce)
cajbir.bugzilla: review+
bugzilla: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
ogg file for exploit.html (1.82 MB, application/octet-stream)
2011-08-13 11:40 PDT, sczimmer
no flags Details

Description User image sczimmer 2011-08-01 14:41:07 PDT
Created attachment 549931 [details]

User Agent: Mozilla/5.0 (X11; U; Linux x86_64; c) AppleWebKit/531.2+ (KHTML, like Gecko) Safari/531.2+ Epiphany/2.30.6

Steps to reproduce:

ran firefox on an .ogg file

it seems like firefox crashes on any ogg file where the beginning of stream flag is set in the second page of a stream, i attached one example

Actual results:

firefox seg faults

Expected results:

firefox should just not play the video like it normally does for corrupted videos rather than seg faulting
Comment 1 User image Mats Palmgren (:mats) 2011-08-01 15:07:03 PDT
Created attachment 549941 [details]

Stack from a recent debug build on Linux64.
Comment 2 User image Chris Pearce (:cpearce) 2011-08-01 16:30:57 PDT
Created attachment 549965 [details] [diff] [review]
Patch v1

The problem is that we're creating a new nsOggCodecState for every bos page we encounter in the stream, and storing a mapping from stream serialno to the nsOggCodecState in the mCodecStates hash table. There are two bos pages for the Vorbis stream. When we create the second nsOggCodecState and add it to mCodecState, we overwrite the original seiral-to-nsOggCodecState mapping in mCoedcStates. mCodecStates holds the owning reference, so the memory pointed to be mVorbisState is then cleared, and when we try to use mVorbisState at nsOggReader.cpp:288 we crash.

So I propose we simply don't create a new nsOggCodecState for serialnos we've seen before in bos pages. The media plays correctly then.
Comment 3 User image cajbir (:cajbir) 2011-08-01 18:56:43 PDT
Comment on attachment 549965 [details] [diff] [review]
Patch v1

Review of attachment 549965 [details] [diff] [review]:

Can you add a test? r- for the main thread/decode thread issue.

::: content/media/ogg/nsOggReader.cpp
@@ +195,5 @@
>      nsOggCodecState* codecState = 0;
> +    if (!ogg_page_bos(&page)) {
> +      // We've encountered the a non Beginning Of Stream page. No more
> +      // BOS pages can follow in this Ogg segment, so there will be no other

The first sentence in the comment seems grammatically incorrect. Should the 'the' be there?

@@ +198,5 @@
> +      // We've encountered the a non Beginning Of Stream page. No more
> +      // BOS pages can follow in this Ogg segment, so there will be no other
> +      // bitstreams in the Ogg (unless it's invalid).
> +      readAllBOS = PR_TRUE;
> +    } else if (!IsKnownStream(ogg_page_serialno(&page))) {

The comment for IsKnownStream says this can be called on the main thread only, but this call is inside ReadMetadata which is called on the decode thread.
Comment 4 User image Chris Pearce (:cpearce) 2011-08-03 14:18:13 PDT
Created attachment 550507 [details] [diff] [review]
Patch v2

Add test and use the decode-thread hash table instead of IsKnownStream.
Comment 5 User image Johnny Stenback (:jst, 2011-08-04 13:19:31 PDT
Chris, I think we're fairly well done with 6 by now, so I won't be tracking this for 6, but if you feel it's critical that we get this into 6 then please nominate the patch for beta approval.
Comment 6 User image Chris Pearce (:cpearce) 2011-08-04 13:48:51 PDT
Ok, cool. This isn't exploitable, so it's not a big deal if we don't get this onto beta at this stage. I'll request aurora approval once it's stuck on central.

Comment 7 User image sczimmer 2011-08-04 15:13:20 PDT
I believe this is exploitable, in most files that trigger the crash the free'd memory is not given to anything, but in the example.ogg I attached, for some reason the free'd mVorbisState buffer is then always allocated in the theora decoder at huffdec.c:399 "tree=(ogg_int16_t *)_ogg_malloc(size*sizeof(*tree));"

in particular what is stored in the important part of the buffer is "leaf=(ogg_int16_t)-(_tokens[ti][1]-depth[l]<<8|_tokens[ti][0]);" in example.ogg leaf is 0xfbfc and when I run the normal linux 32 bit binary from the mozilla download page it crashes with instruction pointer = this address:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xe56ffb70 (LWP 5514)]
0xfbfcfbfc in ?? ()

I believe _tokens comes from the video file so we should have some control over the value. I don't think its even necessary to have complete control over the value, just make it something a bit less than 0xfbfcfbfc and you can do a standard javascript heap spray
Comment 8 User image Chris Pearce (:cpearce) 2011-08-04 16:35:50 PDT
(In reply to comment #7)
> I believe this is exploitable, in most files that trigger the crash the
> free'd memory is not given to anything, but in the example.ogg I attached,
> for some reason the free'd mVorbisState buffer is then always allocated in
> the theora decoder at huffdec.c:399 "tree=(ogg_int16_t
> *)_ogg_malloc(size*sizeof(*tree));"

Just so I understand this, are you saying that exploit happens if we don't crash in ReaderHeaders()? The memory pointed to by mVorbisState gets reallocated in the Theora decoder setup, and then we overwrite the Theora decode info when we init mVorbisState and then the exploit happens when we use the Theora decode info later to to decode some Theora data pages subsequently?

That sounds... plausible to me...

CCing Tim Terriberry, libtheora maintainer, in case he has any insights or can convince me otherwise...
Comment 9 User image Timothy B. Terriberry (:derf) 2011-08-04 17:24:19 PDT
(In reply to comment #8)
> CCing Tim Terriberry, libtheora maintainer, in case he has any insights or
> can convince me otherwise...

The attacker control of that value is somewhat limited: the smallest non-zero value you can get is 0xf8e1f8e1. Bytes 0 and 2 will always be either between 0xe1 and 0xff, or 0x00 (_tokens[ti][0] will be between 0 and 31, inclusive), and bytes 1 and 3 will always be between 0xf8 and 0xff, or 0x00 (nbits will always be between 1 and 7, inclusive, and _tokens[ti][1]-depth[l] is <= nbits). The only way to get _tokens[ti][1]==depth[l] (i.e., to get 0x00 in one of the odd bytes) is with a singleton tree, in which case you will write exactly 32 bits into the buffer, both odd bytes will be 0x00, and both even bytes will also have to be 0x00 (or carry propagation would make the odd bytes 0xff). So the only possible value less than 0xf8e1f8e1 is 0x00000000.

But I don't know enough to say that that would make this unexploitable.
Comment 10 User image sczimmer 2011-08-05 10:04:06 PDT
To clarify,
1. mVorbisState is free'd
2. theora mallocs some memory and recieves the same location mVorbisState was
3. theora modifies the memory, which you have some control over as tim describes above. If you can only make the value > 0xf8e1f8e1 it should still be fine, just a bit less convenient as you will need to place shellcode in the stack instead of the heap
4. mVorbisState->Init() is called. I do not know c++ internals, but apparently that equals the assembly instruction call [rax+0x30] where rax comes from the mVorbisState buffer and is the previously mentioned value from theora we have some control over

This is in 64bit linux firefox 5, the "recent linux64 debug build" above looks like it crashes in a slightly different place

So 64 bit linux crashes due to a read violation on the instruction call [0xfbfcfbfcfbfcfbfc + 0x30], but on 32 bit linux (I am not sure why) it actually crashes with the instruction pointer at address $eip = 0xfbfcfbfc

Of course the only real way to show it's exploitable is with an actual exploit. I'm busy this weekend but will try to write one next week.
Comment 11 User image Chris Pearce (:cpearce) 2011-08-05 11:57:28 PDT
Fixed on central.
Comment 12 User image Chris Pearce (:cpearce) 2011-08-08 14:29:51 PDT
Comment on attachment 550507 [details] [diff] [review]
Patch v2

Requesting aurora approval. Still not sure if this is exploitable, so haven't requested beta approval.
Comment 14 User image Johnathan Nightingale [:johnath] 2011-08-09 14:30:04 PDT
Comment on attachment 550507 [details] [diff] [review]
Patch v2

Land this on aurora this week please, before the migration
Comment 15 User image Chris Pearce (:cpearce) 2011-08-09 16:12:13 PDT
Comment 16 User image Daniel Veditz [:dveditz] 2011-08-10 15:00:29 PDT
fwiw a slightly different (similar) crash with Fx6 Beta:
Comment 17 User image Daniel Veditz [:dveditz] 2011-08-10 15:08:54 PDT
This is a regression from something, Firefox 3.6.x is not affected by the testcase and the code being patched doesn't exist AFAICT.
Comment 18 User image sczimmer 2011-08-13 11:37:14 PDT
I finally wrote an exploit for this bug. Download the attached exploit.html and exploit.ogg in the same folder and visit exploit.html in firefox. I have only coded the exploit for 32 bit linux, and while it might work with random distro's firefox binary I have only tested it with the official mozilla 32 bit linux download firefox-5.0.1. Currently the exploit just prints out "hi!" but of course you can replace the code for printing "hi!" with something more malicious. It works about 90% of the time for me with address space layout randomization off and a bit under 50% of the time with address space layout randomization on though I'm sure that could be improved by someone better at exploit writing than me.
Comment 19 User image sczimmer 2011-08-13 11:38:06 PDT
Created attachment 552888 [details]
exploit for 32 bit linux firefox-5.0.1
Comment 20 User image sczimmer 2011-08-13 11:40:52 PDT
Created attachment 552889 [details]
ogg file for exploit.html
Comment 21 User image Chris Pearce (:cpearce) 2011-08-14 18:38:22 PDT
Comment on attachment 550507 [details] [diff] [review]
Patch v2

Someone more qualified than I needs to make a call on whether this exploit warrants respinning the beta/release-candidate builds, so nominating for beta approval.

* Exploit has only been demonstrated on 32bit Linux, and is less than 100% reproducibility, though higher may be possible.
* Exploit relies on the system malloc returning the memory allocated to a Vorbis decoder (which was incorrectly freed, but to which we still hold a pointer) when allocating a Theora decoder. The Vorbis decoder is initialized in that memory (using setup data read from an ogg file), which overwrites the Theora decoder's memory. When the Theora decoder decodes data, because its setup data is corrupted, the exploit happens.
* Not sure if this can happen on non-Linux platforms, it relies on system malloc behaviour (not jemalloc behaviour).
* Patch is landed in aurora, so if we don't take this on beta, the fix will reach users in 6 more weeks.
Comment 22 User image sczimmer 2011-08-14 20:11:55 PDT
Just to clarify a few things,
While what I said above about using the theora stuff might be possible but this exploit works a much simpler way.
The exploitable bug:
1. A buffer used by ogg decoder is freed.
2. Stuff happens.
3. the instruction call [$eax + 0x18] is executed where $eax is the first word from the freed buffer

Normally the freed memory is garbage so it simply crashes. But, in the exploit.html as the video is started I have looping in the background javascript that rapidly allocates strings of the same size as the buffer that is freed. Almost of all of the time, one of these strings receives the same location in memory as the buffer. We can put whatever we want in the string, so we can control the value of $eax at the instruction call [$eax+0x18], which is straightforward to exploit. I have tested this on windows xp 32 bit and it crashes with attacker control of $eax so it is certainly exploitable on windows as well, I just have a little experience writing exploits for linux and no experience with windows so I wrote it for linux.
Comment 23 User image Daniel Veditz [:dveditz] 2011-08-22 14:56:32 PDT
Comment on attachment 550507 [details] [diff] [review]
Patch v2

We did not respin Firefox 6 for this and the fix is now already in the current "Beta" (Firefox 7) releases.
Comment 24 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-22 16:42:19 PDT
qa+ for verification in Firefox 7 and 8
Comment 25 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-26 10:15:32 PDT
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0) Gecko/20100101 Firefox/7.0 ID:20110922153450
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a2) Gecko/20110926 Firefox/8.0a2 ID:20110926042011
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0 ID:20110922153450
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a2) Gecko/20110926 Firefox/8.0a2 ID:20110926042011
Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20100101 Firefox/7.0 ID:20110922153450
Mozilla/5.0 (X11; Linux x86_64; rv:8.0a2) Gecko/20110926 Firefox/8.0a2 ID:20110926042011

Verified fixed using the attached test video.

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