Closed Bug 499519 Opened 11 years ago Closed 11 years ago

Crash in [@ oggplay_data_handle_theora_frame]

Categories

(Core :: Audio/Video, defect, blocker)

x86
macOS
defect
Not set
blocker

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.1 --- wanted

People

(Reporter: ladamski, Assigned: cpearce)

References

Details

(Keywords: crash, testcase, Whiteboard: [sg:dos])

Crash Data

Attachments

(3 files)

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
how can I reproduce this bug? what's the file that caused this crash?
Attached video testcase
Flags: wanted1.9.1.x+
Flags: wanted1.9.0.x-
Keywords: testcase
Whiteboard: [sg:dos]
Attachment #384351 - Attachment description: repro → testcase
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.
Attached patch Patch v1Splinter Review
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.
Assignee: nobody → chris
Attachment #389041 - Flags: review?(chris.double)
Attachment #389041 - Flags: review?(chris.double) → review+
Attachment #389041 - Flags: superreview?(roc)
Attachment #389041 - Flags: superreview?(roc) → superreview+
Flags: in-testsuite?
Keywords: checkin-needed
Whiteboard: [sg:dos] → [sg:dos][needs landing]
Pushed: http://hg.mozilla.org/mozilla-central/rev/953d5feeb6ca
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attached patch Patch - add testSplinter Review
Patch, adds test to ensure that videos with 0 height load.
Attachment #392408 - Flags: review?(roc)
Comment on attachment 392408 [details] [diff] [review]
Patch - add test

yay!
Attachment #392408 - Flags: review?(roc) → review+
Testcase landed on m-c:
http://hg.mozilla.org/mozilla-central/rev/69611916dcbf
Flags: in-testsuite? → in-testsuite+
Keywords: checkin-needed
Whiteboard: [sg:dos][needs landing] → [sg:dos]
Flags: wanted1.9.1.x+
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.
Actually, is this fixed by the oggplay update on 1.9.1?
Group: core-security
Summary: Crash in oggplay_data_handle_theora_frame → Crash in [@ oggplay_data_handle_theora_frame]
Crash Signature: [@ oggplay_data_handle_theora_frame]
You need to log in before you can comment on or make changes to this bug.