Last Comment Bug 501279 - (CVE-2009-3379) Crash in [@ res0_unpack] at vorbis_res0.c:225
: Crash in [@ res0_unpack] at vorbis_res0.c:225
: crash, testcase, verified1.9.1
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86 Mac OS X
-- critical (vote)
: ---
Assigned To: Matthew Gregan [:kinetik]
: Maire Reavy [:mreavy] Please needinfo me
Depends on:
Blocks: 499512 504644 506094 515889
  Show dependency treegraph
Reported: 2009-06-29 19:52 PDT by Matthew Gregan [:kinetik]
Modified: 2011-06-13 10:01 PDT (History)
14 users (show)
dveditz: wanted1.9.0.x-
cpearce: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (2.31 KB, video/ogg)
2009-06-29 19:52 PDT, Matthew Gregan [:kinetik]
no flags Details
patch v0 (592.46 KB, patch)
2009-07-30 22:41 PDT, Matthew Gregan [:kinetik]
dveditz: approval1.9.1.4+
Details | Diff | Splinter Review
Patch - add test (175.17 KB, patch)
2009-08-03 22:38 PDT, Chris Pearce (:cpearce)
no flags Details | Diff | Splinter Review

Description User image Matthew Gregan [:kinetik] 2009-06-29 19:52:36 PDT
Created attachment 385938 [details]

#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

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.
Comment 1 User image Matthew Gregan [:kinetik] 2009-07-07 19:56:02 PDT
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.
Comment 2 User image Monty Montgomery (:xiphmont) 2009-07-07 21:05:06 PDT
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.
Comment 3 User image Samuel Sidler (old account; do not CC) 2009-07-13 15:47:36 PDT
Not going to block for this, but we should take this when we can.
Comment 4 User image Monty Montgomery (:xiphmont) 2009-07-13 18:47:29 PDT
the pattern review was completed and 1.2.3 released several days ago.
Comment 5 User image Chris Pearce (:cpearce) 2009-07-23 17:19:58 PDT
#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.
Comment 6 User image Monty Montgomery (:xiphmont) 2009-07-23 17:23:46 PDT
Unfortunately, this was en entirely different bug due to a different pattern error.
Comment 7 User image Timothy B. Terriberry (:derf) 2009-07-23 18:04:05 PDT
That was caught by me trying to answer my own questions about how the code worked for 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).
Comment 8 User image Matthew Gregan [:kinetik] 2009-07-30 22:41:07 PDT
Created attachment 391795 [details] [diff] [review]
patch v0

Update libvorbis to r16335.
Comment 9 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2009-08-02 21:56:53 PDT
Comment 10 User image Chris Pearce (:cpearce) 2009-08-03 22:38:36 PDT
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.
Comment 11 User image Chris Pearce (:cpearce) 2009-08-03 23:24:42 PDT
Pushed patch with testcase to m-c:
Comment 12 User image Daniel Veditz [:dveditz] 2009-08-07 12:27:02 PDT
Code freeze for is Tuesday Aug 11, could this be gotten in by then? If so please request approval on the patch.
Comment 13 User image Samuel Sidler (old account; do not CC) 2009-08-10 17:13:47 PDT
I'm guessing the answer is "no". We'll have to get this for
Comment 14 User image cajbir (:cajbir) 2009-08-10 17:19:29 PDT
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?
Comment 15 User image Samuel Sidler (old account; do not CC) 2009-08-10 17:39:09 PDT
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.
Comment 16 User image Daniel Veditz [:dveditz] 2009-09-14 17:54:07 PDT
The tree has been open for, 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)
Comment 17 User image cajbir (:cajbir) 2009-09-14 17:59:35 PDT
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 than to cherry-pick individual commits and hope that we don't break Vorbis somehow.
Comment 18 User image Daniel Veditz [:dveditz] 2009-09-16 15:53:38 PDT
Comment on attachment 391795 [details] [diff] [review]
patch v0

Approved for, a=dveditz for release-drivers
Comment 19 User image Matthew Gregan [:kinetik] 2009-09-17 19:30:18 PDT
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.
Comment 20 User image Daniel Veditz [:dveditz] 2009-09-21 17:26:42 PDT
That's fine -- the approval granted here was on the assumption that we'd end up matching a particular upstream version of the library.
Comment 21 User image Matthew Gregan [:kinetik] 2009-09-21 19:09:19 PDT
Comment 22 User image Al Billings [:abillings] 2009-09-29 11:22:21 PDT
Verified for using testcase with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv: Gecko/20090924 Shiretoko/3.5.4pre. Crashes easily.

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