368427 - Crash [@nsThebesImage::AllowedImageSize, nsThebesImage::Draw] due to "Integer divide by zero"

Status: VERIFIED FIXED

(Core :: Graphics, defect, P1)

Description

[Ryan Jones-Ward [:sciguyryan]]

I've reproduced this bug a new times (see talkbacks).

http://lxr.mozilla.org/mozilla/source/gfx/src/thebes/nsThebesImage.h#128 is reported in the talkback's as being the source of the "Integer divide by zero" error.

TB28759235W, TB28759210Z, TB28759062M, TB28759048K, TB28759016E

Comment 1

[Ryan Jones-Ward [:sciguyryan]]

My guess is that:

NS_ASSERTION(aWidth > 0, "invalid image width");
NS_ASSERTION(aHeight > 0, "invalid image height");

Should be:

NS_ENSURE_SUCCESS(aWidth > 0, PR_FALSE);
NS_ENSURE_SUCCESS(aHeight > 0, PR_FALSE);

Unless its possible to have a 0 width and or a 0 height image.

Comment 2

[Ryan Jones-Ward [:sciguyryan]]

(In reply to comment #1)
> My guess is that:
> 
> NS_ENSURE_SUCCESS(aWidth > 0, PR_FALSE);
> NS_ENSURE_SUCCESS(aHeight > 0, PR_FALSE);
> 
> Unless its possible to have a 0 width and or a 0 height image.
> 

Meant to use NS_ENSURE_TRUE sorry :)

Comment 3

[Ryan VanderMeulen [:RyanVM]]

This should be marked dependent on bug 367740 by someone who has rights to do so. Also, it's been reproduced on Linux as well.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

I'm seeing this on linux too. Both aDWidth and aDHeight are zero in the Draw
call:

0xb59efbc0 in nsThebesImage::Draw (this=0x8e44d48, aContext=@0x8e44be4,
aSurface=0x8e44c48, aSX=<value optimized out>, 
    aSY=<value optimized out>, aSWidth=165, aSHeight=119, aDX=601, aDY=417,
aDWidth=0, aDHeight=0)
    at /home/chb/mozilla/gfx/src/thebes/nsThebesImage.h:128

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

must be because of the aDWidth/aDHeight assignment here in nsThebesImage.cpp:

370       // use '+ 1 - *scale' to get rid of rounding errors
371       aDWidth  = (PRInt32)((srcRect.width)*xscale + 1 - xscale);
372       aDHeight = (PRInt32)((srcRect.height)*yscale + 1 - yscale);

Comment 7

[Mats Palmgren (inactive)]

Patch coming up...

Comment 8

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

(gdb) p mDecoded
$1 = {x = 0, y = 0, width = 0, height = 0}

when we IntersectRect() with that we end up with aDHeight=0 which causes the
division by zero in AllowedImageSize().

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Patch v1 fixes the crashing for me it appears

Comment 11

[Mats Palmgren (inactive)]

Checked in to trunk at 2007-01-27 15:35 PST

-> FIXED

Comment 13

[Steffen Wilberg]

Adding nsThebesImage::AllowedImageSize to summary to prevent more dupes.

Comment 14

[Stephanie Daugherty [:sdaugherty]]

Was seeing this (TB28799332), disappeared with the latest nightly. -> VERIFIED

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

There is at least one crash in TB after this fix:
http://talkback-public.mozilla.org/search/start.jsp?search=2&type=iid&id=28833178
Note the build id 2007012904

nsThebesImage::AllowedImageSize  [mozilla\gfx\src\thebes\nsthebesimage.h, line 128]
nsThebesImage::Draw  [mozilla\gfx\src\thebes\nsthebesimage.cpp, line 383]

(Before the fix, it was line 379, not 383)

Comment 17

[Mats Palmgren (inactive)]

(In reply to comment #16)
> (Before the fix, it was line 379, not 383)

Good catch Olli, thanks.  Apparently there is a nsThebesImage::Draw() call
with aDHeight == 0.  As far as I understand this code, that shouldn't be
allowed and the caller is to blame.  I haven't been able to reproduce the
bug yet though so I don't where the call came from...

Comment 18

[Mats Palmgren (inactive)]

Attachment: Additional patch

This should take care of it for now...

Comment 19

[Mats Palmgren (inactive)]

Comment on attachment 253344 [details] [diff] [review]
Additional patch

Checked in to trunk at 2007-01-30 14:52 PST

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

There is still something wrong :(
http://talkback-public.mozilla.org/search/start.jsp?search=2&type=iid&id=28930489
BuildID 2007020104

nsThebesImage::AllowedImageSize  [mozilla\gfx\src\thebes\nsthebesimage.h, line 128]
nsThebesImage::Draw  [mozilla\gfx\src\thebes\nsthebesimage.cpp, line 388]

Comment 21

[Mats Palmgren (inactive)]

Attachment: Third patch

Let's wallpaper the issue once and for all.
Hopefully someone will spot the assertions in a debug build someday and
file a bug on it...

Comment 22

[Mats Palmgren (inactive)]

Comment on attachment 253765 [details] [diff] [review]
Third patch

Checked in to trunk at 2007-02-08 05:52 PST