Last Comment Bug 672789 - (CVE-2011-2992) this script reliably crashes Firefox 5 on Linux, Mac and Windows by streaming an ogg file through a proxy
: this script reliably crashes Firefox 5 on Linux, Mac and Windows by streaming...
: crash, reproducible, testcase, verified-aurora, verified-beta
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 5 Branch
: All All
-- critical (vote)
: ---
Assigned To: Chris Pearce (:cpearce)
: Maire Reavy [:mreavy] Please needinfo me
Depends on: 652721
  Show dependency treegraph
Reported: 2011-07-20 07:10 PDT by ahu
Modified: 2014-06-26 09:37 PDT (History)
13 users (show)
rforbes: sec‑bounty+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

a little proxy-server that feeds the video-file slowly (3.14 MB, application/octet-stream)
2011-07-20 07:49 PDT, Theo Snelleman
no flags Details
Patch: minimal fix (4.53 KB, patch)
2011-07-20 21:32 PDT, Chris Pearce (:cpearce)
kinetik: review+
Details | Diff | Splinter Review
more stacks (21.47 KB, text/plain)
2011-07-21 15:24 PDT, Mats Palmgren (:mats)
no flags Details

Description User image ahu 2011-07-20 07:10:44 PDT
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.
Comment 1 User image Mats Palmgren (:mats) 2011-07-20 07:30:08 PDT
Please let us know the Crash ID for the crashes you submitted.
Comment 2 User image Ali Juma [:ajuma] 2011-07-20 07:36:17 PDT
Also, please provide a link to the ogg video and/or script that causes the crash.
Comment 3 User image Theo Snelleman 2011-07-20 07:49:14 PDT
Created attachment 547095 [details]
a little proxy-server that feeds the video-file slowly

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.
Comment 4 User image Thomas Ahlblom 2011-07-20 13:11:34 PDT
Moving Report IDs from the Crash Signature field, and adding the signatures there instead.
Comment 5 User image Mats Palmgren (:mats) 2011-07-20 14:45:46 PDT
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?
Comment 6 User image Chris Pearce (:cpearce) 2011-07-20 14:52:55 PDT
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.
Comment 7 User image Chris Pearce (:cpearce) 2011-07-20 15:20:26 PDT
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.
Comment 8 User image Mats Palmgren (:mats) 2011-07-20 15:32:07 PDT
Bug 462960 changes an existing scriptable API (nsIDOMHTMLMediaElement).
Does our process allow that on mozilla-aurora and -beta ?
Comment 9 User image Chris Pearce (:cpearce) 2011-07-20 16:01:40 PDT
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.
Comment 10 User image Chris Pearce (:cpearce) 2011-07-20 21:32:06 PDT
Created attachment 547320 [details] [diff] [review]
Patch: minimal fix

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.
Comment 11 User image Matthew Gregan [:kinetik] 2011-07-20 22:05:34 PDT
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?
Comment 12 User image Daniel Veditz [:dveditz] 2011-07-21 13:36:15 PDT
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.
Comment 13 User image Johnny Stenback (:jst, 2011-07-21 13:36:55 PDT
I think we should take this minimal fix for branches.
Comment 14 User image Mats Palmgren (:mats) 2011-07-21 15:24:09 PDT
Created attachment 547540 [details]
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...)
Comment 15 User image Chris Pearce (:cpearce) 2011-07-21 16:55:11 PDT
(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.
Comment 16 User image Chris Pearce (:cpearce) 2011-07-25 19:17:30 PDT
Fixed on central, aurora, and beta by landing of bug 652721.
Comment 17 User image Chris Pearce (:cpearce) 2011-07-25 19:20:15 PDT
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.
Comment 18 User image Johnny Stenback (:jst, 2011-07-26 09:12:32 PDT
No worries, thanks for landing the fix!
Comment 19 User image ahu 2011-07-26 09:15:07 PDT
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.

Comment 21 User image Daniel Veditz [:dveditz] 2011-08-08 15:20:05 PDT
Is this crash a regression? I can't find the affected code on the 1.9.2 branch.
Comment 22 User image Chris Pearce (:cpearce) 2011-08-08 15:23:59 PDT
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].
Comment 23 User image Chris Pearce (:cpearce) 2011-08-08 15:25:22 PDT
(In reply to Chris Pearce [:cpearce] from comment #22)
> Yes, this is a regression in 5.0.

Sorry, I meant 4.0.
Comment 24 User image Daniel Veditz [:dveditz] 2011-08-08 15:35:40 PDT
"unaffected" is a better status
Comment 25 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-22 16:33:32 PDT
qa+ for verification in Firefox 7 and 8
Comment 26 User image juan becerra [:juanb] 2011-09-26 17:51:09 PDT
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.
Comment 27 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-26 20:09:30 PDT
Verified fixed based on comment 26.

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