696390 - (CVE-2012-0823) Crash [@ vp8_filter_block1d4_v6_ssse3 ] when jumping through HTML5-Video

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Johannes Pfrang [:johnp]]

I experienced this crash when I was trying to jump to another point of the video on the given URL. First time happend using yesterdays nightly and with restarting Firefox (x64) updated and I could reproduce the problem. I'll try to reproduce it without any Add-Ons enabled now. I'm sorry for filling as General, but I couldn't find a suitable component. Please move if you know one ;)

STR:
1. Open the URL
2. Start the video as HTML5 (default)
3. Try to jump to another point of time in the video. Probably needs more than one try.

Crashes:

bp-a6d16837-8f84-4d19-b0f7-cf2d72111021
bp-d5f9452a-8f74-4bc5-a637-9eaae2111021
bp-5feb5184-13bc-44a1-9dd1-5c18a2111021 (somehow the 2nd one was sent two times)

Stack (for the 2nd one):

0 	xul.dll 	vp8_filter_block1d4_v6_ssse3 	
1 	xul.dll 	xul.dll@0x61cb6b 	
2 	xul.dll 	vp8_sixtap_predict4x4_ssse3 	media/libvpx/vp8/common/x86/vp8_asm_stubs.c:548
3 	nspr4.dll 	MD_CURRENT_THREAD 	nsprpub/pr/src/md/windows/w95thred.c:308
4 	xul.dll 	vp8_build_inter_predictors_b 	media/libvpx/vp8/common/reconinter.c:143
5 	xul.dll 	vp8_build_inter4x4_predictors_mb 	media/libvpx/vp8/common/reconinter.c:349
6 	xul.dll 	vp8_build_inter_predictors_mb 	media/libvpx/vp8/common/reconinter.c:380
7 	xul.dll 	decode_macroblock 	media/libvpx/vp8/decoder/decodframe.c:237
8 	xul.dll 	decode_mb_row 	media/libvpx/vp8/decoder/decodframe.c:447
9 	xul.dll 	vp8_decode_frame 	media/libvpx/vp8/decoder/decodframe.c:1109
10 	nspr4.dll 	MD_CURRENT_THREAD 	nsprpub/pr/src/md/windows/w95thred.c:308
11 	xul.dll 	vp8dx_receive_compressed_data 	media/libvpx/vp8/decoder/onyxd_if.c:416
12 	mozutils.dll 	arena_malloc_small 	memory/jemalloc/jemalloc.c:3820
13 	xul.dll 	vp8_decode 	media/libvpx/vp8/vp8_dx_iface.c:454
14 	mozalloc.dll 	moz_xmalloc 	memory/mozalloc/mozalloc.cpp:105
15 	xul.dll 	nsWebMReader::NextPacket 	content/media/webm/nsWebMReader.cpp:566
16 	xul.dll 	vpx_codec_decode 	media/libvpx/vpx/src/vpx_decoder.c:138
17 	xul.dll 	nsWebMReader::DecodeVideoFrame 	content/media/webm/nsWebMReader.cpp:683
18 	d3d10_1core.dll 	CLayeredObject<NMultithread::CTexture2D>::`vector deleting destructor' 	
19 	msvcrt.dll 	free 	
20 	mozutils.dll 	arena_dalloc_small 	memory/jemalloc/jemalloc.c:4241
21 	nspr4.dll 	MD_CURRENT_THREAD 	nsprpub/pr/src/md/windows/w95thred.c:308
22 	nspr4.dll 	MD_CURRENT_THREAD 	nsprpub/pr/src/md/windows/w95thred.c:308
23 	xul.dll 	nsBuiltinDecoderReader::DecodeToTarget 	content/media/nsBuiltinDecoderReader.cpp:274
24 	xul.dll 	nsThread::Shutdown 	xpcom/threads/nsThread.cpp:484
25 	xul.dll 	nsWebMReader::Seek 	content/media/webm/nsWebMReader.cpp:767
26 	xul.dll 	nsBuiltinDecoderStateMachine::DecodeSeek 	content/media/nsBuiltinDecoderStateMachine.cpp:1333
27 	xul.dll 	nsBuiltinDecoderStateMachine::DecodeThreadRun 	content/media/nsBuiltinDecoderStateMachine.cpp:310
28 	xul.dll 	nsRunnableMethodImpl<unsigned int 	obj-firefox/dist/include/nsThreadUtils.h:345
29 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:631
30 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:347
31 	xul.dll 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:245
32 	xul.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:272
33 	nspr4.dll 	PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:426
34 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:122
35 	msvcr90.dll 	endthreadex 	
36 	msvcr90.dll 	endthreadex 	
37 	msvcr90.dll 	get_fpsr 	
38 	kernel32.dll 	kernel32.dll@0x1652c 	
39 	ntdll.dll 	RtlUserThreadStart 	
40 	kernel32.dll 	kernel32.dll@0x992ef 	
41 	kernel32.dll 	kernel32.dll@0x992ef

Comment 1

[Alice0775 White]

Regression window(m-c)
Works;
http://hg.mozilla.org/mozilla-central/rev/9545b88eed82
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111014 Firefox/10.0a1 ID:20111014030948
Crashes;
http://hg.mozilla.org/mozilla-central/rev/ca73f057dab7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111014 Firefox/10.0a1 ID:20111014030040
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9545b88eed82&tochange=ca73f057dab7

Regression window(m-i)
Works;
http://hg.mozilla.org/integration/mozilla-inbound/rev/5f16d668ce5a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111013 Firefox/10.0a1 ID:20111013172738
Crashes;
http://hg.mozilla.org/integration/mozilla-inbound/rev/c7542ce9069a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111013 Firefox/10.0a1 ID:20111013174038
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5f16d668ce5a&tochange=c7542ce9069a
Triggerd by:
Bug 693057 - Update libvpx to v-0.9.7-p1

Comment 2

[Timothy B. Terriberry (:derf)]

So far I'm not having a lot of luck reproducing this (it doesn't crash in 64-bit Linux, my development environment). It's failing in a SPLIT_MV macroblock where the block-level MV in the Y plane is out of bounds. This is an area of the code we've had trouble with before, but I don't see anything obvious in the changes to it that would cause this (there were some recent bugs introduced here upstream, but after the v0.9.7-p1 version, and they've already been fixed). I'll keep digging, though, and I've asked for help getting this caught in a debugger in Windows.

Comment 3

[Johannes Pfrang [:johnp]]

Attachment: WinDbg Nightly x64 log (e79245e249c4)

I could help if you can give me some instructions of what I have to do. I don't have that much experience in debugging and it seems that my own debug build somehow didn't have any symbols built or used by WinDbg... So I tried the instructions from "https://developer.mozilla.org/en/How_to_get_a_stacktrace_with_WinDbg" with my official x64 nightly and attached the results. 
Questions: Could I use a tinderbox debug build to get better results and can the corresponding symbols be downloaded from mozilla symbol server? How can I build a debug build with symbols on my local machine (I found a bug about symbol build failing on x64 (bug 669384))?

Comment 4

[Alice0775 White]

Attachment: WinDbg log of 32bit Nightly on Wndows7 64bit PC

Comment 5

[Johannes Pfrang [:johnp]]

Attachment: WinDbg log of debug x86 build

I managed to compile a debug build with pdb-files, but somehow "make buildsymbols" didn't work so I had to manually add the pdb's to the symbol search path. Stack differs slightly (one line ahead).

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Reproduced on a trunk m-c build on Win7.

An earlier attempt once produced numerous assertions when seeking around in the file.  These probably should lead to filing other bugs unless these are just the file being stupid (in which case I'm not sure an assertion is the correct response).

WARNING: Audio not synced after seek, maybe a poorly muxed file?: file c:/mozilla/head/obj-debug2/content/media/../../../content/media/nsBuiltinDecoderReader.cpp, line 350
###!!! ASSERTION: Seek target should lie inside the first audio block after seek: '!audio || (audio->mTime <= seekTime && seekTime <= audio->mTime + audio->mDuration)', file c:/mozilla/head/obj-debug2/content/media/../..../content/media/nsBuiltinDecoderStateMachine.cpp, line 1339
###!!! ASSERTION: Should know audio start time if we have audio.: '!HasAudio() || mAudioStartTime != -1', file c:/mozilla/head/obj-debug2/content/media/../../../content/media/nsBuiltinDecoderStateMachine.cpp, line 1658
###!!! ASSERTION: Should know audio start time if we have audio.: '!HasAudio() |
| mAudioStartTime != -1', file c:/mozilla/head/obj-debug2/content/media/../../..
/content/media/nsBuiltinDecoderStateMachine.cpp, line 1658
###!!! ASSERTION: Should have audio start time by now: 'audioStartTime != -1', f
ile c:/mozilla/head/obj-debug2/content/media/../../../content/media/nsBuiltinDec
oderStateMachine.cpp, line 511
###!!! ASSERTION: Should know audio start time if we have audio.: '!HasAudio() |
| mAudioStartTime != -1', file c:/mozilla/head/obj-debug2/content/media/../../..
/content/media/nsBuiltinDecoderStateMachine.cpp, line 1658
###!!! ASSERTION: Clock should go forwards: 'mCurrentFrameTime <= clock_time', f
ile c:/mozilla/head/obj-debug2/content/media/../../../content/media/nsBuiltinDec
oderStateMachine.cpp, line 1682
###!!! ASSERTION: Should know audio start time if we have audio.: '!HasAudio() |
| mAudioStartTime != -1', file c:/mozilla/head/obj-debug2/content/media/../../..
/content/media/nsBuiltinDecoderStateMachine.cpp, line 1658
###!!! ASSERTION: Clock should go forwards: 'mCurrentFrameTime <= clock_time', f
ile c:/mozilla/head/obj-debug2/content/media/../../../content/media/nsBuiltinDec
oderStateMachine.cpp, line 1682
###!!! ASSERTION: Buffered range must end after its start: 'startOffset < endOff
set', file c:/mozilla/head/obj-debug2/content/media/../../../content/media/nsMed
iaCache.cpp, line 2301

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Those assertion failures are unrelated to this bug, and should be dealt with in a separate bug.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Reproduced in a debugger: 

Unhandled exception at 0x777e15ee in firefox.exe: 0xC0000005: Access violation reading location 0x123dff43

on the stack: 
 	xul.dll!vp8_sixtap_predict4x4_ssse3(unsigned char * src_ptr=0x123dff45, int src_pixels_per_line=800, int xoffset=6, int yoffset=0, unsigned char * dst_ptr=0x0ba35248, int dst_pitch=16)  Line 539 + 0x11 bytes

So, it looks like it accessed 2 bytes before the start of the src pointer, but even the input src ptr may have been bad:
+		src_ptr	0x123dff45 <Bad Ptr>	unsigned char *

One layer up:
>	xul.dll!build_inter_predictors4b(MacroBlockD * x=0x0ba34f20, BLOCKD * d=0x00000000, int pitch=16)  Line 177 + 0x1b bytes

d=NULL doesn't look good, but it's a little hard to be sure if that' real.  The bbb array in build_inter4x4_predictors_mb() is 0,2,8,10, and BLOCKD = &x->block[bbb[i]], and i < 4.  But you couldn't really get 0 in any case.  However, there are obviously some inlinings going on.

But, assuming it's unable to show 'd' due to the optimizer, I looked at x.block[0,2,8,10].

The base_pre's all look reasonable

d->pre is 0, 8, 6400, 6408

d->bmi.as_mode is 0xfff2fff8, 0xffeefef8, 0xfff2fff8, 0xfff2fff8  -- seems odd...  may be unset

The d->bmi.mv.as_mv.rol,col are: -8, -14   -264, -18,   -8, -14,    -8, -14

I will note that x.block[24] has a 0 value for ->predictor and ->base_pre


The MACROBLOCKD structure:
-		x	0x0ba34f20 {diff=0x0ba34f20 predictor=0x0ba35240 
etc - looks good as best I can tell

Comment 9

[John Koleszar]

Attachment: fix libvpx splitmv clamping

This patch likely fixes the crash, but this behavior shouldn't be invoked on well formed streams. Seek bug maybe?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Tried a number of times; could not crash it with the patch applied (crashed before without it)

Comment 11

[Timothy B. Terriberry (:derf)]

Committed upstream as https://gerrit.chromium.org/gerrit/10925

Comment 12

[Timothy B. Terriberry (:derf)]

Attachment: Fix libvpx splitmv clamping v2

I went ahead and cleaned up jk8's patch (so, e.g., it uses 8 lines of context and updates update.sh).

Greenish on try: https://tbpl.mozilla.org/?tree=Try&rev=813987b01c21

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 571451 [details] [diff] [review]
Fix libvpx splitmv clamping v2

Perhaps we should try to get this on aurora and beta as well?

Comment 14

[Timothy B. Terriberry (:derf)]

(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #13)
> Perhaps we should try to get this on aurora and beta as well?

It should only affect v0.9.7, which just landed recently in bug 693057, so aurora and beta shouldn't be affected. It was broken in an upstream commit from May, and we hadn't updated the decoder since then.

Comment 15

[Timothy B. Terriberry (:derf)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc3fe56d732d

Comment 16

[Matthew Gregan [:kinetik]]

(In reply to John Koleszar from comment #9)
> This patch likely fixes the crash, but this behavior shouldn't be invoked on
> well formed streams. Seek bug maybe?

The Clusters pointed to by the Cues in the linked video start with with P-frames.  We rely on the decoder to deal with this rather than explicitly skipping forward to the first I-frame before starting to decode.

Comment 17

[Timothy B. Terriberry (:derf)]

(In reply to Matthew Gregan [:kinetik] from comment #16)
> The Clusters pointed to by the Cues in the linked video start with with
> P-frames.  We rely on the decoder to deal with this rather than explicitly
> skipping forward to the first I-frame before starting to decode.

Perhaps we should pass an aSkipToKeyframe flag to DecodeToTarget, and set it after calling nestegg_track_seek(). It'd probably also be useful to set after seeking in Ogg, and in nsBuiltinDecoderReader::DecodeVideoFrame() (so that DecodeToFirstData skips to a keyframe for live streams). In fact the only time you don't want to use it is when seeking in Ogg if you decide the target is already close enough to the current position.

In theory there's nothing wrong with shoving random packets into the decoder (it needs to be robust enough to handle it), but it would be faster to skip decoding them, and showing garbage at the beginning of a live stream isn't a great user experience, either.

Comment 18

[Matthew Gregan [:kinetik]]

Sounds good, filed bug 699316.

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

[written before Tim's latest comment, but submitting as-is for now]
Was the problem that the decoder would barf on a stream that starts with pframes?  If so.... ewwww :-)

Seriously: I would assume that until first IDR one would simply not generate an output frame, OR would generate a degraded (probably very degraded/predator-mode) frame with a flag indicating lack of an IDR, perhaps depending on the application/options.

I assume part of the tests for vp8 is to (effectively) pipe /dev/random into it... :-)

Comment 20

[Randell Jesup [:jesup] (needinfo me)]

I'll note that in videophone/chat use one might miss the initial IDR/keyFrame and so be presented with a stream starting with pFrames for an unknown time. Also, I suppose one could program an encoder to do "rolling" refresh (AIR refresh) and thus the decoder might never see an IDR/keyFrame if it missed the initial one, but after a bounded period of time the screen would be error-free.  Since I assume* that vp8 specifies the decoder, it's possible someone might write such an encoder.

*I realize what "assume" implies.  :-)  I also realize the "AIR" refresh is pretty ugly, though it does help with generating a flat bitrate.   (I believe AIR stands for Adaptive Intra-Refresh)

Comment 21

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/bc3fe56d732d

Comment 22

[Daniel Veditz [:dveditz]]

This bug was apparently assigned CVE-2012-0823 by Google.