The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in mozilla1.9.3a1

Status

()

Core
Audio/Video
P2
critical
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: keeler, Assigned: kinetik)

Tracking

(4 keywords)

Trunk
mozilla1.9.3a1
crash, testcase, verified1.9.1, verified1.9.2
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 +
wanted1.9.0.x -
in-testsuite ?

Firefox Tracking Flags

(status1.9.2 beta1-fixed, blocking1.9.1 .4+, status1.9.1 .4-fixed)

Details

(Whiteboard: [sg:critical?])

Attachments

(3 attachments, 1 obsolete attachment)

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
Created attachment 391363 [details]
stack trace of relevant thread
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

8 years ago
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.

Comment 5

8 years ago
decoder_example segfaults for me. vorbisfile_example doesn't.
Bug poke: is this being worked on?
Fixed by bug 512328.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
bug 512328 was backed out due to bug 513999 :(
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
Depends on: 512328, 513999
(Reporter)

Updated

8 years ago
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
Last Resolved: 8 years ago8 years ago
status1.9.2: --- → beta1-fixed
Resolution: --- → FIXED
Flags: blocking1.9.2? → blocking1.9.2+
blocking1.9.1: ? → .4+
status1.9.1: --- → wanted
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.
status1.9.1: wanted → ?
status1.9.2: beta1-fixed → ---
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 → ---
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.
Assignee: nobody → chris
Attachment #403951 - Flags: review?(chris.double)

Updated

8 years ago
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?
(Assignee)

Comment 16

8 years ago
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?
Sounds reasonable.
(Assignee)

Comment 18

8 years ago
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.
Attachment #403951 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
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.
status1.9.1: ? → wanted
(Assignee)

Comment 20

8 years ago
http://hg.mozilla.org/mozilla-central/rev/196956e36ed2
Assignee: chris → kinetik
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
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+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d21357a3baf1
status1.9.2: --- → beta1-fixed
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
(Assignee)

Comment 25

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/40e2100dcefe
status1.9.1: wanted → .4-fixed
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.