499519 - Crash in [@ oggplay_data_handle_theora_frame]

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Lucas Adamski [:ladamski]]

SIGFPE, looks to be a div by zero

#0	0x13555c37 in oggplay_data_handle_theora_frame at oggplay_data.c:370
#1	0x1355447b in oggplay_callback_theora at oggplay_callback.c:201
#2	0x1356204e in oggz_read_sync at oggz_read.c:483
#3	0x13562459 in oggz_read at oggz_read.c:606
#4	0x13553b95 in oggplay_step_decoding at oggplay.c:691
#5	0x13540fec in nsOggDecodeStateMachine::DecodeFrame at nsOggDecoder.cpp:732
#6	0x13546683 in nsOggDecodeStateMachine::Run at nsOggDecoder.cpp:1428
#7	0x005759e4 in nsThread::ProcessNextEvent at nsThread.cpp:510
#8	0x004fe968 in NS_ProcessNextEvent_P at nsThreadUtils.cpp:227
#9	0x00575bf3 in nsThread::ThreadFunc at nsThread.cpp:254
#10	0x00728465 in _pt_root at ptthread.c:228
#11	0x96291155 in _pthread_start
#12	0x96291012 in thread_start

Comment 1

[Viktor Gal]

how can I reproduce this bug? what's the file that caused this crash?

Comment 2

[Lucas Adamski [:ladamski]]

Attachment: testcase

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

https://bugzilla.mozilla.org/show_bug.cgi?id=474077 looks like this bug, but with an older version of the code.

Comment 5

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

Pretty clear integer division by zero. The Theora track's header declares the frame height to be 0 pixels, and then divides something by that.

Comment 6

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

Attachment: Patch v1

Disable theora track with y-height or uv-height of 0. Prevents the division by 0, since tracks with 0 y-height won't be played, hence the divisor in the crashing code should never be 0.

The sound track still plays.

Also wrapped the uv_offset calculation code in some divisor checking code, so that if a part of the calculation rounds down to 0 (e.g. 1/2 = 0) we still don't divide by 0. I'm not sure if we should disable the track in this case, as it's got non-zero y,uv sizes.

Comment 7

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

Pushed: http://hg.mozilla.org/mozilla-central/rev/953d5feeb6ca

Comment 8

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

Attachment: Patch - add test

Patch, adds test to ensure that videos with 0 height load.

Comment 9

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

Comment on attachment 392408 [details] [diff] [review]
Patch - add test

yay!

Comment 10

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

Testcase landed on m-c:
http://hg.mozilla.org/mozilla-central/rev/69611916dcbf

Comment 11

[Daniel Veditz [:dveditz]]

Are we going to fix this in Firefox 3.5 (1.9.1)? If not--it's just a DoS--we should mark the status1.9.1 field "wontfix" and unhide the bug. Otherwise please request 1.9.1 approval on the patch you would like to land.

Comment 12

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

Actually, is this fixed by the oggplay update on 1.9.1?