Closed Bug 507167 Opened 15 years ago Closed 15 years ago

crash (segfault) @ _make_words when playing corrupted ogg vorbis file

Categories

(Core :: Audio/Video, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta1-fixed
blocking1.9.1 --- .4+
status1.9.1 --- .4-fixed

People

(Reporter: keeler, Assigned: kinetik)

References

Details

(4 keywords, Whiteboard: [sg:critical?])

Attachments

(3 files, 1 obsolete file)

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
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?
Interestingly it doesn't crash the vorbisfile_decode example in libvorbis.
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.
decoder_example segfaults for me. vorbisfile_example doesn't.
Bug poke: is this being worked on?
Fixed by bug 512328.
Status: UNCONFIRMED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
bug 512328 was backed out due to bug 513999 :(
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
Whiteboard: [sg:critical?]
Status: UNCONFIRMED → NEW
Ever confirmed: true
blocking1.9.1: --- → ?
Flags: blocking1.9.2?
Fixed on trunk and 1.9.2 by bug 512328 checkin.
Status: NEW → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Flags: blocking1.9.2? → blocking1.9.2+
blocking1.9.1: ? → .4+
Flags: wanted1.9.0.x-
This was *not* fixed on 1.9.2 or 1.9.1 by the ogg libraries update. It is fixed on trunk though.
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. :(
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Patch v1 (obsolete) — Splinter Review
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.
Assignee: nobody → chris
Attachment #403951 - Flags: review?(chris.double)
Attachment #403951 - Flags: review?(chris.double) → review+
Need to add the test file as a test?
Priority: -- → P2
Whiteboard: [sg:critical?] → [sg:critical?][needs landing]
(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...
okay, let's make it in-testsuite? and check in the test later.
Flags: in-testsuite?
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?
Attached patch patch v2Splinter Review
Update libvorbis to SVN r16597.  Fixes this bug and bug 518941.  We want this on all three branches.
Attachment #403951 - Attachment is obsolete: true
Blocks: 518941
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.
http://hg.mozilla.org/mozilla-central/rev/196956e36ed2
Assignee: chris → kinetik
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Attachment #404188 - Flags: approval1.9.2?
Attachment #404188 - Flags: approval1.9.1.4?
Target Milestone: --- → mozilla1.9.3a1
blocking1.9.1: .4+ → .5+
Comment on attachment 404188 [details] [diff] [review]
patch v2

We'll have to pick up this upgrade in 1.9.1.5
Attachment #404188 - Flags: approval1.9.1.4? → approval1.9.1.5?
Attachment #404188 - Flags: approval1.9.2? → approval1.9.2+
blocking1.9.1: .5+ → .4+
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
Attachment #404188 - Flags: approval1.9.1.5? → approval1.9.1.4+
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
Severity: normal → critical
Status: RESOLVED → VERIFIED
Keywords: verified1.9.2
OS: Linux → All
Hardware: x86 → All
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
Keywords: verified1.9.1
Whiteboard: [sg:critical?][needs landing] → [sg:critical?]
Group: core-security
You need to log in before you can comment on or make changes to this bug.