500254 - (CVE-2009-2663) Crash in vorbis_book_decodevv_add at vorbis_codebook.c:483

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Lucas Adamski [:ladamski]]

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

Comment 1

[Lucas Adamski [:ladamski]]

Attachment: testcase

Comment 2

[Lucas Adamski [:ladamski]]

Attachment: testcase multiple - not all null derefs but all low memory addresses

Comment 3

[Lucas Adamski [:ladamski]]

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
	  if(chptr==ch){
	    chptr=0;
	    i++;
	  }
	}

Since a is an array of pointers (I have no idea where its subsequently used)  exploitability cannot be ruled out.

Comment 4

[Lucas Adamski [:ladamski]]

Sorry, to clarify this could result in a[chptr++][i] being potentially set to an attacker controlled value.

Comment 5

[cajbir (:cajbir)]

Do these files need the crc disabled to exhibit the crash?

Comment 6

[cajbir (:cajbir)]

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.

Comment 7

[Monty Montgomery (:xiphmont)]

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.

Comment 8

[Monty Montgomery (:xiphmont)]

Fixes are in place as commits 16181 and 16182.

Comment 9

[cajbir (:cajbir)]

Attachment: Update to libvorbis containing fix

This updates to the libvorbis svn revision containing the fixes

Comment 10

[cajbir (:cajbir)]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/f46e6aee1335

Comment 11

[Samuel Sidler (old account; do not CC)]

Have we seen any regressions from this libvorbis update? 300k patches scare me, but... is this ready for 1.9.1?

Comment 12

[Monty Montgomery (:xiphmont)]

Apologies; a whitespace cleanup patch got pushed in the meantime.  changes to 1.2.3 were not extensive.

Comment 13

[cajbir (:cajbir)]

There have been no regressions from this patch landing on trunk that I know of.

Comment 14

[cajbir (:cajbir)]

Comment on attachment 385289 [details] [diff] [review]
Update to libvorbis containing fix

On discussion with Robert I'll cherry pick the relevant commits for 1.9.1.1 since they're small.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Make that 1.9.1.2 - this will have to wait for the next release due to a firedrill.

Comment 16

[cajbir (:cajbir)]

Attachment: Fix for 1.9.1 branch

Contains just the fix cherrypicked from libvorbis svn

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Does this need review or is it ready for approval?

Comment 18

[cajbir (:cajbir)]

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.

Comment 19

[Samuel Sidler (old account; do not CC)]

Comment on attachment 388596 [details] [diff] [review]
Fix for 1.9.1 branch

Approved for 1.9.1.2. a=ss for release-drivers

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/08e95ed9d135

Comment 21

[juan becerra [:juanb]]

Verified with examples in comment #2 using: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2

3.5 crashed. 3.5.2 did not crash with any of the several ogg samples I tried.

Comment 22

[Matthew Gregan [:kinetik]]

Comment on attachment 385289 [details] [diff] [review]
Update to libvorbis containing fix

Requesting approval for this based on https://bugzilla.mozilla.org/show_bug.cgi?id=501279#c19

Comment 23

[Daniel Veditz [:dveditz]]

Comment on attachment 385289 [details] [diff] [review]
Update to libvorbis containing fix

Approved for 1.9.1.4, a=dveditz

Comment 24

[Matthew Gregan [:kinetik]]

Backed out cherrypicked patch:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a23de485f6db

Landed 'Update to libvorbis containing fix' that is on trunk:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bbf4a1e5d9af

Comment 25

[Matthew Gregan [:kinetik]]

This bug is showing up on the list of things that need landing for 1.9.1.4 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?

Comment 26

[Daniel Veditz [:dveditz]]

it could go either way, but since we should get this reverified let's go with updating the fix version.

Comment 27

[Tracy Walker [:tracy]]

verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.4pre) Gecko/20091001 Shiretoko/3.5.4pre