Last Comment Bug 507167 - crash (segfault) @ _make_words when playing corrupted ogg vorbis file
: crash (segfault) @ _make_words when playing corrupted ogg vorbis file
Status: VERIFIED FIXED
[sg:critical?]
: crash, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: All All
: P2 critical (vote)
: mozilla1.9.3a1
Assigned To: Matthew Gregan [:kinetik]
:
: Maire Reavy [:mreavy]
Mentors:
Depends on: CVE-2009-3378 513999
Blocks: 518941
  Show dependency treegraph
 
Reported: 2009-07-29 09:57 PDT by David Keeler [:keeler] (use needinfo?)
Modified: 2009-11-09 18:35 PST (History)
16 users (show)
roc: blocking1.9.2+
dveditz: wanted1.9.0.x-
roc: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed
.4+
.4-fixed


Attachments
test file that causes crash (8.00 KB, audio/ogg)
2009-07-29 09:57 PDT, David Keeler [:keeler] (use needinfo?)
no flags Details
stack trace of relevant thread (3.01 KB, text/plain)
2009-07-29 09:58 PDT, David Keeler [:keeler] (use needinfo?)
no flags Details
Patch v1 (2.63 KB, patch)
2009-09-30 21:19 PDT, Chris Pearce (:cpearce)
cajbir.bugzilla: review+
Details | Diff | Splinter Review
patch v2 (61.98 KB, patch)
2009-10-01 21:19 PDT, Matthew Gregan [:kinetik]
roc: approval1.9.2+
dveditz: approval1.9.1.4+
Details | Diff | Splinter Review

Description David Keeler [:keeler] (use needinfo?) 2009-07-29 09:57:57 PDT
Created attachment 391362 [details]
test file that causes crash

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.12) Gecko/2009070811 Ubuntu/9.04 (jaunty) Firefox/3.0.12
Build Identifier: mozilla-central revision bf4d4c37361c

Segmentation fault when playing corrupted ogg vorbis file (attached).

Reproducible: Always

Steps to Reproduce:
1. Load attached file.
Actual Results:  
firefox crashes

Expected Results:  
some sort of "this file is corrupted" message
Comment 1 David Keeler [:keeler] (use needinfo?) 2009-07-29 09:58:21 PDT
Created attachment 391363 [details]
stack trace of relevant thread
Comment 2 Chris Pearce (:cpearce) 2009-07-29 14:59:47 PDT
Hmm, this file also crashes a stand alone ogg program I'm writing, it looks like a bug in libvorbis, not libfishsound. It's crashing in vorbis_synthesis_init() after the last header packet is read, Monty, can you take a look?
Comment 3 cajbir (:cajbir) 2009-07-29 21:33:58 PDT
Interestingly it doesn't crash the vorbisfile_decode example in libvorbis.
Comment 4 Chris Pearce (:cpearce) 2009-07-29 21:59:34 PDT
I checked out http://svn.xiph.org/trunk/vorbis and used that, and I still crash, so this isn't fixed (on Windows at least) by the latest trunk.

The decoder_example program crashed for me with vorbis trunk, I'm using libogg 1.1.4 stable, on Windows.
Comment 5 cajbir (:cajbir) 2009-07-29 22:01:28 PDT
decoder_example segfaults for me. vorbisfile_example doesn't.
Comment 6 David Keeler [:keeler] (use needinfo?) 2009-08-26 11:43:13 PDT
Bug poke: is this being worked on?
Comment 7 Chris Pearce (:cpearce) 2009-08-30 22:52:25 PDT
Fixed by bug 512328.
Comment 8 Chris Pearce (:cpearce) 2009-09-01 22:03:01 PDT
bug 512328 was backed out due to bug 513999 :(
Comment 9 Chris Pearce (:cpearce) 2009-09-27 01:24:33 PDT
Fixed on trunk and 1.9.2 by bug 512328 checkin.
Comment 10 Chris Pearce (:cpearce) 2009-09-30 13:50:34 PDT
This was *not* fixed on 1.9.2 or 1.9.1 by the ogg libraries update. It is fixed on trunk though.
Comment 11 Chris Pearce (:cpearce) 2009-09-30 14:19:43 PDT
This is not fixed in trunk nightlies either! For some reason this is fixed in my local opt build from 2009-09-27, but not in a local debug or any other nightly build. This appearing fixed in my local build is probably why I thought it was fixed by the ogg libraries update in the first place. Reopening. :(
Comment 12 Chris Pearce (:cpearce) 2009-09-30 21:19:33 PDT
Created attachment 403951 [details] [diff] [review]
Patch v1

We worked with the vorbis developers, and they commited r16957 upstream to fix this. Here's that patch cherry picked. It's a simple 1 line fix which adds input validation to the vorbis header decoding. We don't crash loading the test file on trunk with this patch.
Comment 13 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-10-01 07:30:50 PDT
Need to add the test file as a test?
Comment 14 Chris Pearce (:cpearce) 2009-10-01 13:06:59 PDT
(In reply to comment #13)
> Need to add the test file as a test?

I'm never sure whether to includes test cases in security patches for fear of exposing some exploit to the bad guys. Under what conditions should we not include test cases? When the bug is exploitable? How can I tell? This test file causes stuff on the stack to be overwritten, but I'm unsure if it's exploitable...
Comment 15 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-10-01 16:14:12 PDT
okay, let's make it in-testsuite? and check in the test later.
Comment 16 Matthew Gregan [:kinetik] 2009-10-01 20:21:05 PDT
Rather than take this as a cherrypicked patch, I think we should just update to libvorbis SVN.  There's another fix we want for trunk (and should take on 1.9.2 and 1.9.1) in bug 518941, which also cherrypicks the patch from SVN... but these two changes are the only significant changes in SVN since we took the last update.  So I think it'd keep things simpler to sync with SVN again rather than carrying more patches.

Opinions?
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-10-01 21:11:13 PDT
Sounds reasonable.
Comment 18 Matthew Gregan [:kinetik] 2009-10-01 21:19:33 PDT
Created attachment 404188 [details] [diff] [review]
patch v2

Update libvorbis to SVN r16597.  Fixes this bug and bug 518941.  We want this on all three branches.
Comment 19 Daniel Veditz [:dveditz] 2009-10-02 11:33:39 PDT
OK, go for it on the 1.9.1 branch.(In reply to comment #14)

> (In reply to comment #13)
> I'm never sure whether to includes test cases in security patches for fear of
> exposing some exploit to the bad guys. Under what conditions should we not
> include test cases? When the bug is exploitable?

We've 0day'd ourselves by checking in security bug regression tests on the trunk in the past (a security bug that turned WFM w/out investigation what fixed it, but with the release branches still vulnerable). Check in the test when the bug is fixed on all supported branches that are vulnerable. Preferably request approval on the old branches when you've made the fix, you don't need to wait for the branch drivers to notice and flag the bug as "wanted".

If you don't know if the old branches are vulnerable then assume they are until you've tested.

> How can I tell? This test file causes stuff on the stack to be
> overwritten, but I'm unsure if it's exploitable...

Assume it is exploitable. It's not worth the effort to prove that in a given specific instance there are enough mitigations to make it safe.
Comment 20 Matthew Gregan [:kinetik] 2009-10-04 16:34:37 PDT
http://hg.mozilla.org/mozilla-central/rev/196956e36ed2
Comment 21 Daniel Veditz [:dveditz] 2009-10-05 14:44:12 PDT
Comment on attachment 404188 [details] [diff] [review]
patch v2

We'll have to pick up this upgrade in 1.9.1.5
Comment 22 Chris Pearce (:cpearce) 2009-10-05 20:02:31 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d21357a3baf1
Comment 23 Daniel Veditz [:dveditz] 2009-10-06 11:02:41 PDT
Comment on attachment 404188 [details] [diff] [review]
patch v2

not particularly happy rushing this in at the last minute, but it's safer than not fixing it given the upstream's "exploit me here" comment :-(

Approved for 1.9.1.4, a=dveditz for release-drivers
Comment 24 Henrik Skupin (:whimboo) 2009-10-06 14:38:08 PDT
Verified fixed on trunk and 1.9.2 with builds on all platforms like:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091006 Minefield/3.7a1pre ID:20091006031035

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20091006 Namoroka/3.6b1pre ID:20091006034008
Comment 25 Matthew Gregan [:kinetik] 2009-10-06 15:32:39 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/40e2100dcefe
Comment 26 Henrik Skupin (:whimboo) 2009-10-08 11:46:46 PDT
Verified fixed on 1.9.1 with builds on Windows and OS X like Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.4) Gecko/20091006 Firefox/3.5.4 ID:20091006224018

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