Crash in [@ oggplay_data_handle_theora_frame]

RESOLVED FIXED

Status

()

Core
Audio/Video
--
blocker
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: ladamski, Assigned: cpearce)

Tracking

({crash, testcase})

Trunk
x86
Mac OS X
crash, testcase
Points:
---
Bug Flags:
wanted1.9.0.x -
in-testsuite +

Firefox Tracking Flags

(status1.9.1 wanted)

Details

(Whiteboard: [sg:dos], crash signature)

Attachments

(3 attachments)

(Reporter)

Description

9 years ago
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

9 years ago
how can I reproduce this bug? what's the file that caused this crash?
(Reporter)

Comment 2

9 years ago
Created attachment 384351 [details]
testcase
Flags: wanted1.9.1.x+
Flags: wanted1.9.0.x-
Keywords: testcase
Whiteboard: [sg:dos]
(Reporter)

Updated

9 years ago
Attachment #384351 - Attachment description: repro → testcase
Duplicate of this bug: 504472
https://bugzilla.mozilla.org/show_bug.cgi?id=474077 looks like this bug, but with an older version of the code.
(Assignee)

Comment 5

9 years ago
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.
(Assignee)

Comment 6

9 years ago
Created attachment 389041 [details] [diff] [review]
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.
Assignee: nobody → chris
Attachment #389041 - Flags: review?(chris.double)

Updated

9 years ago
Attachment #389041 - Flags: review?(chris.double) → review+
(Assignee)

Updated

9 years ago
Attachment #389041 - Flags: superreview?(roc)
Attachment #389041 - Flags: superreview?(roc) → superreview+
(Assignee)

Updated

9 years ago
Flags: in-testsuite?
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [sg:dos] → [sg:dos][needs landing]
status1.9.1: --- → wanted
(Assignee)

Comment 7

9 years ago
Pushed: http://hg.mozilla.org/mozilla-central/rev/953d5feeb6ca
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

9 years ago
Created attachment 392408 [details] [diff] [review]
Patch - add test

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+
(Assignee)

Comment 10

9 years ago
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

Updated

8 years ago
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.