Bug 501279 (CVE-2009-3379)

Crash in [@ res0_unpack] at vorbis_res0.c:225

RESOLVED FIXED

Status

()

Core
Audio/Video
--
critical
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: kinetik, Assigned: kinetik)

Tracking

({crash, testcase, verified1.9.1})

Trunk
x86
Mac OS X
crash, testcase, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.0.x -
in-testsuite +

Firefox Tracking Flags

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

Details

(Whiteboard: [sg:critical?], crash signature)

Attachments

(3 attachments)

(Assignee)

Description

8 years ago
Created attachment 385938 [details]
testcase

Stack:
#0  res0_unpack at vorbis_res0.c:225
#1  _vorbis_unpack_books at vorbis_info.c:313
#2  vorbis_synthesis_headerin at vorbis_info.c:425
#3  fs_vorbis_decode at fishsound_vorbis.c:131
#4  fish_sound_decode at fishsound_decode.c:117
#5  oggplay_callback_audio at oggplay_callback.c:392
#6  oggz_read_sync at oggz_read.c:483
#7  oggz_read at oggz_read.c:606
#8  oggplay_initialise at oggplay.c:122

Locals:
225	    if(ci->book_param[info->booklist[j]]->maptype==0)goto errout;
(gdb) p j
$6 = 0
(gdb) p info->booklist[j]
$7 = -1
(gdb) p ci->book_param[info->booklist[j]]
$8 = (static_codebook *) 0x0

Crashes trunk with the Vorbis update from bug 500254 applied.  Also crashes decoder_example from Vorbis SVN trunk.
(Assignee)

Updated

8 years ago
Whiteboard: [sg:investigate?]
Flags: wanted1.9.1.x+
Flags: blocking1.9.1.1?
(Assignee)

Comment 1

8 years ago
Fixed in Subversion r16218 upstream, but it sounds like Monty plans to conduct a pattern review for this, so we should probably wait before picking up a new version.
Yes, pattern review plus a new release soon.  There's also a small-file open bug that Mozilla doesn't care about but is important for gaming developers, so there will be a 1.2.3 shortly.
Not going to block 1.9.1.1 for this, but we should take this when we can.
Flags: blocking1.9.1.1?
the pattern review was completed and 1.2.3 released several days ago.
#vorbis, 24/7/09:
[12:06]	<CIA-28>	xiphmont * r16326 vorbis/lib/backends.h: Eliminate possibility of booklist overflow in res0/1/2 unpacking.
[12:07]	<derf>	cpearce: You guys may be interested in that r16326 commit.

This must be the result of Monty's pattern review which Matthew mentioned in comment #1.
Unfortunately, this was en entirely different bug due to a different pattern error.
That was caught by me trying to answer my own questions about how the code worked for https://bugzilla.mozilla.org/show_bug.cgi?id=506094. The issue, is, however, entirely unrelated to the issue found by that file (or this file), and should be tracked as a separate bug. A fuzz tester would've had to get almost 200 bits in a row exactly right to trigger it, but a sentient attacker would've had no trouble producing a heap overflow, writing up to 128 words past the end of a block with the low 8 bits set to attacker-chosen values (and the high bits cleared).
(Assignee)

Comment 8

8 years ago
Created attachment 391795 [details] [diff] [review]
patch v0

Update libvorbis to r16335.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
(Assignee)

Updated

8 years ago
Assignee: kinetik → nobody
Whiteboard: [sg:investigate?] → [sg:investigate?][needs landing]
http://hg.mozilla.org/mozilla-central/rev/c6692a8f3f27
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [sg:investigate?][needs landing] → [sg:investigate?]
Flags: in-testsuite?
Created attachment 392431 [details] [diff] [review]
Patch - add test

Adds test that ensures that the testcase doesn't load, and throws an error. Also includes the testcase from bug 504644 and bug 506094 as well, as this bug fixes those bug's testcase as too. This is rebased on the testcase patches for bug 500311, bug 498855 and bug 499519.
Pushed patch with testcase to m-c:
http://hg.mozilla.org/mozilla-central/rev/5e68517728d2
Flags: in-testsuite? → in-testsuite+
Blocks: 504644
Blocks: 506094
blocking1.9.1: --- → ?
status1.9.1: --- → wanted
Flags: wanted1.9.1.x+
Whiteboard: [sg:investigate?] → [sg:critical?]
Keywords: crash, testcase
blocking1.9.1: ? → .3+
Code freeze for 1.9.1.3 is Tuesday Aug 11, could this be gotten in by then? If so please request approval on the patch.
Assignee: nobody → chris
I'm guessing the answer is "no". We'll have to get this for 1.9.1.4...
blocking1.9.1: .3+ → .4+

Comment 14

8 years ago
Chris Pearce is currently away. The patch is an update to libvorbis - 500Kb or so. In a previous point release it was suggested to cherry pick the actual change to fix the bug since people were nervous about approving a 500Kb patch. Is that still the case?
I think we're "okay" with taking an update to libvorbis if it's well-baked/tested and lands right when the 1.9.1 tree opens. Ideally we'd cherry pick all fixes, but I'd hate to miss some fixes because we didn't take a full update. Likewise, there might be ogg changes we *want* but aren't security issues in such an update.
The tree has been open for 1.9.1.4, in fact code freeze is in a week (Sept 22). Whatever we're going to do we need to do ASAP.

Yes, we're nervous about a 600K patch, but can you safely cherry pick out the fixes for this bug, the two that were duped to it, and some of the new ones that have come in recently? (e.g. bug 515889)
Blocks: 515889

Comment 17

8 years ago
The patch is large due to white space and formatting changes in the Vorbis library. It has been trunk for about 6 weeks now without problems. I think it is safer to land this patch, which is known to work, on 1.9.1.4 than to cherry-pick individual commits and hope that we don't break Vorbis somehow.

Updated

8 years ago
Attachment #391795 - Flags: approval1.9.1.4?
Assignee: chris → kinetik
Comment on attachment 391795 [details] [diff] [review]
patch v0

Approved for 1.9.1.4, a=dveditz for release-drivers
Attachment #391795 - Flags: approval1.9.1.4? → approval1.9.1.4+
(Assignee)

Comment 19

8 years ago
So, the bad news is that this patch depends on the earlier libvorbis update from bug 500254.  We cherrypicked the necessary bits for the 1.9.1 branch, which is why this patch won't apply to 1.9.1.

If we want this on 1.9.1, then I think the best course of action is to back out the cherrypicked fixes on 1.9.1 and apply the complete patches from bug 500254 and here.
That's fine -- the approval granted here was on the assumption that we'd end up matching a particular upstream version of the library.
(Assignee)

Comment 21

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e140b6415b4a
status1.9.1: wanted → .4-fixed
Verified for 1.9.1.4 using testcase with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.4pre) Gecko/20090924 Shiretoko/3.5.4pre. Crashes 1.9.1.3 easily.
Keywords: verified1.9.1
Blocks: 499512
Alias: CVE-2009-3379
Group: core-security

Updated

8 years ago
Severity: normal → critical
Summary: Crash in res0_unpack at vorbis_res0.c:225 → Crash in [@ res0_unpack] at vorbis_res0.c:225
Flags: wanted1.9.0.x-
Crash Signature: [@ res0_unpack]
You need to log in before you can comment on or make changes to this bug.