Closed Bug 1279348 Opened 3 years ago Closed 3 years ago

Division by 0 in mozilla::VideoInfo::ScaledImageRect

Categories

(Core :: Audio/Video: Playback, defect, P1, critical)

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- wontfix
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: mccr8, Assigned: jya)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-f2f10e9b-f15f-479b-9f01-88b422160608.
=============================================================

This is not a common crash (I only see 6 in crash-stats), but maybe it is easy to fix. I assume that either mImage.width or mImage.height is 0.
Crash volume for signature 'mozilla::VideoInfo::ScaledImageRect':
 - nightly (version 50): 79 crashes from 2016-06-06.
 - aurora  (version 49): 0 crash from 2016-06-07.
 - beta    (version 48): 1 crash from 2016-06-06.
 - release (version 47): 3 crashes from 2016-05-31.
 - esr     (version 45): 0 crash from 2016-04-07.

Crash volume on the last weeks:
             Week N-1   Week N-2   Week N-3   Week N-4   Week N-5   Week N-6   Week N-7
 - nightly         22          6          1          6         19         23          2
 - aurora           0          0          0          0          0          0          0
 - beta             0          1          0          0          0          0          0
 - release          1          0          2          0          0          0          0
 - esr              0          0          0          0          0          0          0

Affected platform: Windows
Comment on attachment 8777622 [details]
Bug 1279348 - Fix guard in scaledImageRect;

https://reviewboard.mozilla.org/r/69152/#review66234

::: dom/media/MediaInfo.h:264
(Diff revision 1)
>    // reports. This is legal in WebM, and we will preserve the ratio of the crop
>    // rectangle as it was reported relative to the picture size reported by the
>    // container.
>    nsIntRect ScaledImageRect(int64_t aWidth, int64_t aHeight) const
>    {
> -    if (aWidth == mImage.width && aHeight == mImage.height) {
> +    if (aWidth == mImage.width || aHeight == mImage.height) {

this will break aspect ratio where only one dimension changed.

Volume is probably low enough to not worry about it but still
Attachment #8777622 - Flags: review?(jyavenard)
Assignee: nobody → jyavenard
Attachment #8777622 - Attachment is obsolete: true
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cdab1a072387
Avoid division by 0. r=kentuckyfriedtakahe
https://hg.mozilla.org/mozilla-central/rev/cdab1a072387
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Could you uplift this to m-b or at least m-a? 

SeaMonkey suite does crash reliably with webm when benchmarking using this page and/or trying to play some webm videos: https://www.youtube.com/html5 . 

I still wonder why Firefox is not affected but the fix seems to cure it.
Hi Jean-Yves, do you think it's worthwhile uplifting this patch to Beta50?
Flags: needinfo?(jyavenard)
Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

Approval Request Comment
[Feature/regressing bug #]:1243538
[User impact if declined]: Crashes (division by 0)
[Describe test coverage new/current, TreeHerder]: In central. Don't know of a way to reproduce it.
[Risks and why]: Low. Those are likely invalid file to start with. We simply check if one of the dimension is 0 and if so don't attempt to scale the image. 
[String/UUID change made/needed]: None
Flags: needinfo?(jyavenard)
Attachment #8791022 - Flags: approval-mozilla-aurora?
Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

This patch fixes a crash(division by 0). Take it in 51 aurora.
Attachment #8791022 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Per comment# 16, it's already in 51.
Hi :jya,
Do you mean uplift to 50?
Flags: needinfo?(jyavenard)
Attachment #8791022 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
I did mean 50.
Flags: needinfo?(jyavenard)
Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

Approval Request Comment
[Feature/regressing bug #]:1243538
[User impact if declined]: Crashes (division by 0)
[Describe test coverage new/current, TreeHerder]: In central. Don't know of a way to reproduce it.
[Risks and why]: Low. Those are likely invalid file to start with. We simply check if one of the dimension is 0 and if so don't attempt to scale the image. 
[String/UUID change made/needed]: None
Attachment #8791022 - Flags: approval-mozilla-beta?
Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

Crash fix, Beta50+
Attachment #8791022 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Thanks for uplifting this.
You need to log in before you can comment on or make changes to this bug.