493224 - Crash [@ oggplay_buffer_release] with malformed ogg file

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Martijn Wargers (dead)]

See url, when viewing that ogg file in the browser, it crashes after a view seconds.

http://crash-stats.mozilla.com/report/index/92afc3ec-5c1d-4ba3-8cc6-6a6bb2090515
0  	xul.dll  	oggplay_buffer_release  	 media/liboggplay/src/liboggplay/oggplay_buffer.c:313
1 	xul.dll 	nsOggDecodeStateMachine::NextFrame 	content/media/video/src/nsOggDecoder.cpp:735
2 	xul.dll 	nsOggDecodeStateMachine::QueueDecodedFrames 	content/media/video/src/nsOggDecoder.cpp:1015
3 	xul.dll 	nsOggDecodeStateMachine::PlayFrame 	content/media/video/src/nsOggDecoder.cpp:820
4 	xul.dll 	nsOggDecodeStateMachine::Run 	content/media/video/src/nsOggDecoder.cpp:1252
5 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
6 	xul.dll 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:230
7 	xul.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:254
8 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:426
9 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:122
10 	mozcrt19.dll 	_callthreadstartex 	obj-firefox/memory/jemalloc/crtsrc/threadex.c:348
11 	mozcrt19.dll 	_threadstartex 	obj-firefox/memory/jemalloc/crtsrc/threadex.c:326
12 	kernel32.dll 	kernel32.dll@0xb728 

I guess it might be related to a couple of the other bugs that are open about this, not sure.

Comment 1

[cajbir (:cajbir)]

Crashes in command line liboggplay based player as well. Added liboggplay maintainer to the CC list.

Comment 2

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

Wouldn't block the release on this, would still take a patch.

Comment 3

[Viktor Gal]

Attachment: drop frames which has smaller granule position than the last seen one

The problem with the given ogg content is that the theora stream is not following the specifications: ogg spec. requires that in any stream the granule position will never decrease. the ogg contents mentioned in bug 463726 are having the same problem. The theora frames' granule position is sometimes decreasing.

The attach patch basically drops the frames which has smaller granule position than the last - 'valid' - seen one. Of course by this the video is not going to be 'consistent' - meaning the video 'freezes' (see football.ogg) -, but it is a badly created content.

Maybe it would be better that in case we detect this behavior we simply stop decoding the content, instead of trying to enforce the decoding in the above mentioned way...

any ideas/suggestions/comments?

Comment 4

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

Either way is fine --- actually, whatever is simplest is best --- but it should actually be specified in the Ogg or Theora specs. We don't want to be in a situation where one player plays the content and another doesn't.

> The problem with the given ogg content is that the theora stream is not
> following the specifications

Yeah, we're talking about content created by bad guys who are trying to take over the browser, who aren't interested in following the specification :-)

Comment 5

[cajbir (:cajbir)]

Attachment: Fix

Viktor's fix from comment 3 packaged up with README_MOZILLA, changes, etc.

Comment 6

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

http://hg.mozilla.org/mozilla-central/rev/2d3e4849df48

We should have a testcase here.

Comment 7

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3545ba557ef9

Comment 8

[Daniel Veditz [:dveditz]]

(In reply to comment #3)
> The problem with the given ogg content is that the theora stream is not
> following the specifications: ogg spec.

Evil hackers make their living violating specs. Expect the unexpected!

Comment 9

[Daniel Veditz [:dveditz]]

The "testcase" keyword means there's a testcase attached to the bug. A URL pointing at a server doesn't count because that can go away and invalidate the keyword.

Comment 10

[Daniel Veditz [:dveditz]]

Attachment: testcase from URL

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with on trunk and 1.9.1:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090526 Minefield/3.6a1pre ID:20090526044156

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090526 Shiretoko/3.5pre (.NET CLR 3.5.30729) ID:20090526042535