Closed Bug 500254 (CVE-2009-2663) Opened 13 years ago Closed 13 years ago

Crash in vorbis_book_decodevv_add at vorbis_codebook.c:483


(Core :: Audio/Video, defect)

Not set



Tracking Status
blocking1.9.1 --- .2+
status1.9.1 --- .4-fixed


(Reporter: ladamski, Assigned: cajbir)


(Keywords: verified1.9.1, Whiteboard: [sg:critical?] specific fix in, library upgrade in


(3 files, 1 obsolete file)

This particular repro appears to be null deref but I have 50+ repros to go through.  The null pointer results from a math operation referencing other objects so...

#0	0x1363afab in vorbis_book_decodevv_add at vorbis_codebook.c:483
#1	0x13637cd9 in res2_inverse at vorbis_res0.c:872
#2	0x13639972 in mapping0_inverse at vorbis_mapping0.c:783
#3	0x1362daf6 in vorbis_synthesis at vorbis_synthesis.c:81
#4	0x135faddc in fs_vorbis_decode at fishsound_vorbis.c:158
#5	0x135fa122 in fish_sound_decode at fishsound_decode.c:117
#6	0x135ffaff in oggplay_callback_audio at oggplay_callback.c:391
#7	0x1360d04e in oggz_read_sync at oggz_read.c:483
#8	0x1360d459 in oggz_read at oggz_read.c:606
#9	0x135feb95 in oggplay_step_decoding at oggplay.c:691
#10	0x135ebfec in nsOggDecodeStateMachine::DecodeFrame at nsOggDecoder.cpp:732
#11	0x135f1683 in nsOggDecodeStateMachine::Run at nsOggDecoder.cpp:1428
#12	0x005759e4 in nsThread::ProcessNextEvent at nsThread.cpp:510
#13	0x004fe968 in NS_ProcessNextEvent_P at nsThreadUtils.cpp:227
#14	0x00575bf3 in nsThread::ThreadFunc at nsThread.cpp:254
#15	0x00728465 in _pt_root at ptthread.c:228
#16	0x96291155 in _pthread_start
#17	0x96291012 in thread_start
Attached video testcase (obsolete) —
Reviewed all of the test cases, several are not null derefs though all are low addresses:
video-1frag.ogg.13.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000078
video-1frag.ogg.14.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000070
video-1frag.ogg.15.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000078
video-1frag.ogg.16.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.17.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000068
video-1frag.ogg.18.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000048
video-1frag.ogg.19.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000078
video-1frag.ogg.20.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000070
video-1frag.ogg.25.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000038
video-1frag.ogg.26.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.27.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000030
video-1frag.ogg.28.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.29.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.30.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.31.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000008
video-1frag.ogg.32.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.38.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x000007f8  <---------
video-1frag.ogg.39.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.40.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000300
video-1frag.ogg.41.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.42.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.43.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
video-1frag.ogg.44.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000010
video-1frag.ogg.45.ogg.log:Reason: KERN_PROTECTION_FAILURE at address: 0x00000000

Offending piece of code is, where t == exception address

	const float *t = book->valuelist+entry*book->dim;
	for (j=0;j<book->dim;j++){
	  a[chptr++][i]+=t[j];  //<---------- crash here

Since a is an array of pointers (I have no idea where its subsequently used)  exploitability cannot be ruled out.
Sorry, to clarify this could result in a[chptr++][i] being potentially set to an attacker controlled value.
Whiteboard: [sg:critical?]
Flags: blocking1.9.1?
Do these files need the crc disabled to exhibit the crash?
I get this crash with these files on a standalone libvorbis based player using latest SVN of libvorbis. So it's probably not a liboggplay or Firefox specific issue. CCing libvorbis maintainer.
Confirmed in SVN.  There are two bugs in play here, both due to the libvorbis setup code not noticing an invalid setup and rejecting the file like it should. I fully understand both bugs and am instrumenting a fix to libvorbis as well as adding the checks to our verification tools. Neither fix will impact performance or other files once in place.
Fixes are in place as commits 16181 and 16182.
Flags: blocking1.9.1? → blocking1.9.1-
Whiteboard: [sg:critical?] → [sg:critical?][3.5.1?]
Flags: wanted1.9.1.x+
This updates to the libvorbis svn revision containing the fixes
Assignee: nobody → chris.double
Flags: blocking1.9.1.1+
Whiteboard: [sg:critical?][3.5.1?] → [sg:critical?]
Pushed to mozilla-central:
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?] → [sg:critical?][baking for]
Have we seen any regressions from this libvorbis update? 300k patches scare me, but... is this ready for 1.9.1?
Apologies; a whitespace cleanup patch got pushed in the meantime.  changes to 1.2.3 were not extensive.
There have been no regressions from this patch landing on trunk that I know of.
Attachment #385289 - Flags: approval1.9.1.1?
Attachment #385289 - Flags: approval1.9.1.1?
Comment on attachment 385289 [details] [diff] [review]
Update to libvorbis containing fix

On discussion with Robert I'll cherry pick the relevant commits for since they're small.
Make that - this will have to wait for the next release due to a firedrill.
blocking1.9.1: --- → .2+
Flags: blocking1.9.1.1+ → blocking1.9.1.1-
Contains just the fix cherrypicked from libvorbis svn
Does this need review or is it ready for approval?
Comment on attachment 388596 [details] [diff] [review]
Fix for 1.9.1 branch

It's ready for approval and doesn't need review. It's cherry picked from the existing change on trunk.
Attachment #388596 - Flags: approval1.9.1.2?
Whiteboard: [sg:critical?][baking for] → [sg:critical?]
Comment on attachment 388596 [details] [diff] [review]
Fix for 1.9.1 branch

Approved for a=ss for release-drivers
Attachment #388596 - Flags: approval1.9.1.2? → approval1.9.1.2+
Verified with examples in comment #2 using: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv: Gecko/20090729 Firefox/3.5.2

3.5 crashed. 3.5.2 did not crash with any of the several ogg samples I tried.
Keywords: verified1.9.1
Group: core-security
Flags: wanted1.9.0.x-
Alias: CVE-2009-2663
Comment on attachment 385289 [details] [diff] [review]
Update to libvorbis containing fix

Requesting approval for this based on
Attachment #385289 - Flags: approval1.9.1.4?
Comment on attachment 385289 [details] [diff] [review]
Update to libvorbis containing fix

Approved for, a=dveditz
Attachment #385289 - Flags: approval1.9.1.4? → approval1.9.1.4+
Backed out cherrypicked patch:

Landed 'Update to libvorbis containing fix' that is on trunk:
This bug is showing up on the list of things that need landing for because of the approval on the earlier patch (which has landed, per comment 24).  Should status1.9.1 be changed from .2-fixed to .4-fixed?
it could go either way, but since we should get this reverified let's go with updating the fix version.
Keywords: verified1.9.1
Whiteboard: [sg:critical?] → [sg:critical?] specific fix in, library upgrade in
verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv: Gecko/20091001 Shiretoko/3.5.4pre
Keywords: verified1.9.1
You need to log in before you can comment on or make changes to this bug.