Closed Bug 672789 (CVE-2011-2992) Opened 13 years ago Closed 13 years ago

this script reliably crashes Firefox 5 on Linux, Mac and Windows by streaming an ogg file through a proxy


(Core :: Audio/Video, defect)

5 Branch
Not set



Tracking Status
firefox5 --- wontfix
firefox6 + fixed
firefox7 + fixed
firefox8 + fixed
status1.9.2 --- unaffected


(Reporter: ahu, Assigned: cpearce)



(6 keywords, Whiteboard: [sg:critical?][qa!])

Crash Data


(3 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/534.30 (KHTML, like Gecko) Chrome/12.0.742.124 Safari/534.30

Steps to reproduce:

Connect to a proxyserver & stream a specific ogg file to an HTML5 player. Then I seek to a part that has not yet been loaded. 

Actual results:

On firefox 4 this works, on 5 and 6 I get a crossplatform crash (Windows, Linux, Mac), a segfault

Expected results:

No segfault.
Please let us know the Crash ID for the crashes you submitted.
Severity: normal → critical
Also, please provide a link to the ogg video and/or script that causes the crash.
Crash Signature: bp-8a8b28c4-3cb3-4e1c-9a28-cddc02110720 bp-e409ab9b-bcd4-4eba-a0ec-d161f2110720
This is a tarball containing a little proxy-server that helped me to reproduce the error.

After starting the proxy-server, let firefox connect to port 8000.
Then browse to

A webcam-video will be shown. The proxy-server will feed the data very slowly, which allows you to seek to the end of the video before it is fully loaded.
When you seek to a part that isn't fetched yet, a segfault will occur.

the proxy-server is replying to the request with HTTP/1.1 200 OK and not with 206 Partial content. No filesize will be transmitted. To report the duration of the video to firefox, the "X-Content-Duration:"-header is used. Maybe this is relevant for this issue.
Moving Report IDs from the Crash Signature field, and adding the signatures there instead.
Crash Signature: bp-8a8b28c4-3cb3-4e1c-9a28-cddc02110720 bp-e409ab9b-bcd4-4eba-a0ec-d161f2110720 → [@ nsCSSRendering::FindBackgroundStyleFrame ] [@ js::PropertyTable::search ]
OS: Other → Linux
Hardware: All → x86_64
Group: core-security
Ever confirmed: true
Whiteboard: [sg:critical?]
I can reproduce this reliably in a local trunk debug build on Linux64.
Before the crash I see this on the console:

###!!! ASSERTION: Guess must be before range end: 'guess < endOffset', file content/media/ogg/nsOggReader.cpp, line 1252
WARNING: NS_ENSURE_TRUE(ret == 0) failed: file content/media/ogg/nsOggReader.cpp, line 1127
WARNING: NS_ENSURE_TRUE(res != PAGE_SYNC_ERROR) failed: file content/media/ogg/nsOggReader.cpp, line 1268
WARNING: NS_ENSURE_SUCCESS(res, res) failed with result 0x80004005: file content/media/ogg/nsOggReader.cpp, line 1066

then I crash with a SEGV in random parts of the code.
Our video code is overwriting random pieces of memory?
Component: General → Video/Audio
Product: Firefox → Core
QA Contact: general →
Bug 462960 should prevent this error by ensuring we never try to seek into ranges which are unbuffered in a resource/connection which doesn't support HTTP byte range requests.
Depends on: 462960
Paul, it would be really great if we could get bug 462960 and its dependencies landed. The changes to the seek algorithm you make in bug 462960 should prevent this crash happening.
Bug 462960 changes an existing scriptable API (nsIDOMHTMLMediaElement).
Does our process allow that on mozilla-aurora and -beta ?
Not sure.

If we can't get bug 462960 on m-a or m-b, we could probably do a simple fix in each of the Ogg/Wave/WebM backend's to not seek into unbuffered ranges if the media isn't reporting that it supports byte range requests.
I suspect that since the patch from bug 462960 changes guids on interfaces wouldn't be wanted on m-{a,b} because it may affect binary add-ons?

Here's a patch to implement the minimal fix, pretty much what I described in comment 9. With this if you try to seek into an unbuffered range, the seek just aborts and playback continues from the previous playback position.
Assignee: nobody → chris
Attachment #547320 - Flags: review?(kinetik)
Comment on attachment 547320 [details] [diff] [review]
Patch: minimal fix

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

::: content/media/nsBuiltinDecoderReader.cpp
@@ +44,5 @@
>  #include "nsBuiltinDecoderReader.h"
>  #include "nsBuiltinDecoderStateMachine.h"
>  #include "mozilla/mozalloc.h"
>  #include "VideoUtils.h"
> +#include "nsTimeRanges.h"

This can be included in the .cpp file.

::: content/media/nsBuiltinDecoderStateMachine.h
@@ +417,5 @@
>    void DecodeSeek();
> +  // Returns PR_TRUE if mSeekTarget is not in a buffered range and we
> +  // cannot seek the media into unbuffered ranges.
> +  PRBool IsSeekTargetUnreachable();

Would you mind renaming this to IsSeekTargetReachable and inverting the result and the test where we use it?
Attachment #547320 - Flags: review?(kinetik) → review+
How is this potentially exploitable? It appears we'd read "ogg data" from uninitialized or unowned memory. If it doesn't crash on an access violation (safe) how is that different than someone feeding us a garbage ogg stream (which shouldn't be exploitable or else we've got worse problems!).

The two crash stacks attached to this bug are both null derefs.
I think we should take this minimal fix for branches.
Attached file more stacks
(In reply to comment #12)
>It appears we'd read "ogg data" from uninitialized or unowned memory.

That's not what I'm seeing using a local debug build on Linux64.
I'm not crashing in the ogg decoder, but some time later in arbitrary
parts of the code.  I looked at more than a dozen crashes and all of
them appeared to have been caused by arbitrary memory being overwritten.
I'm assuming that the bug in the ogg code caused this to happen and I'm
worried that what was written can be influenced by the stream.

Here's a few of the crash stacks.
(JaegerTrampoline() sounds like a bad place to crash...)
(In reply to comment #12)
> How is this potentially exploitable?

In this case we're reading data from file and writing it to memory. Due to unsigned integer overflow, in this case we'll end up writing however much we've downloaded from file into our 8KB memory buffer. Chances are we'll have downloaded more than 8KB by this stage, and we'll end up overflowing the bounds of that buffer and stomping memory with whatever was read from file.

This is caused by unsigned values overflowing at nsOggReader.cpp:1100 (aEndOffset==-1 as the stream's length is unknown). Matthew Gregan picked this up in bug 652721. With his patch applied, when we seek in this case, rather than crashing the media is just rendered unplayable (due the playback engine not recovering from a failed seek correctly, it's not an artefact of his patch).

Ideally we should take Matthew's patch in bug 652721 on central + branches to fix the crash, and then take Paul's patches in bug 462960 on central to prevent us trying to seek into unbuffered ranges when the stream isn't seekable. Paul's patches make my patch in this bug unnecessary, so we may as well take his patches instead.
Fixed on central, aurora, and beta by landing of bug 652721.
Closed: 13 years ago
Depends on: 652721
No longer depends on: 462960
OS: Linux → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Sorry, the tracking flags got reset (maybe a bug in bugzilla helper?). I can't reset the tracking-firefoxN flags, but have reset the other ones.
No worries, thanks for landing the fix!
Hi everybody,

On behalf of Theo and me, thank you very much for resolving this issue so quickly, we are much impressed!

Keep up the great work.

blocking1.9.2: --- → .20+
Is this crash a regression? I can't find the affected code on the 1.9.2 branch.
blocking1.9.2: .20+ → ?
Yes, this is a regression in 5.0. I was unable to reproduce the crash in [Mozilla/4.0 (Windows; U; Windows NT 6.1; en-US; rv: Gecko/20110319 Firefox/3.6.16].
(In reply to Chris Pearce [:cpearce] from comment #22)
> Yes, this is a regression in 5.0.

Sorry, I meant 4.0.
blocking1.9.2: ? → ---
"unaffected" is a better status
Alias: CVE-2011-2992
qa+ for verification in Firefox 7 and 8
Whiteboard: [sg:critical?] → [sg:critical?][qa+]
Verified this on Linux on 6.0.2, 7.0RC(build2), and 8.0a2 using the steps in comment #0 and comment #3. In Fx5 moving the seek handle beyond what it has loaded crashed the application. In the fixed versions it does not crash.
Verified fixed based on comment 26.
Whiteboard: [sg:critical?][qa+] → [sg:critical?][qa!]
Group: core-security
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.