120781 - Mozilla crashes with oversized GIF images

Status: RESOLVED WORKSFORME

(Core :: Graphics: ImageLib, defect)

Description

[Martin Raue]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:0.9.7) Gecko/20011221
BuildID:    2001122106

Mozilla crashs everytime I try to open the url www.amoebius.org/fuehler/01/

Reproducible: Always
Steps to Reproduce:
1. Open www.amoebius.org/fuehler/01/ in Mozilla

Actual Results:  Mozilla crahs

Expected Results:  Mozilla should display the page.

Maybe it's a problem with the html, because the page is extremly old school coded.

Comment 1

[Ludovic Hirlimann]

Works fine in w2k build ID 2002011703. Reporter what build are you using ?
Also please add crash to the keywords and change Severity to Critical ...

Comment 2

[Martin Raue]

My build is 2001122106 (see BuilID in the header of the original bug report). I
added the keyword crash and changed the status to critical.

Comment 3

[Ludovic Hirlimann]

I can confirm with CFM build  2002011808.

Comment 4

[Ludovic Hirlimann]

Attachment: Image found on the site that crashes mozilla

Comment 5

[Ludovic Hirlimann]

Can someone change the component to imagelib ?

Comment 6

[Brian Nesse (gone)]

Opening this is a debug build generates the following message:
###!!! ASSERTION: PixMap too big for QuickDraw: '0', file nsImageMac.cpp line 571.

Hung machine hard, so I haven't looked at it, but i suppose 0 could imply a 0
byte image or something...

Comment 7

[Brian Nesse (gone)]

Ok, this image claims to have dimensions of 6000x100 pixels. This generates a
rowBytes count that is > 0x4000 which causes nsImageMac::CreatePixMap() to
return a paramErr. This error is propagated back up to gfxImageFrame::Init()
which ignores it.

Comment 8

[Brian Nesse (gone)]

It eventually appears to die in nsGIFDecoder2.cpp at line 471 when it memsets
decoder->mRGBLine to 0.

The problem here is two-fold. First, no image memory is allocated, and second,
there appears to be no way to bail from the decoding process if an error occurs.
I believe that QuickDraw now does support larger PixMaps, when using the
accessor methods and opaque structures. This should give us a solution for the
carbon build, but we are still limited on the CFM build. cc'ing some other mac
folks.

Comment 9

[Stuart Parmenter]

Adding GIF error handling bug as a dependancy.

Comment 10

[Stuart Parmenter]

Attachment: Patch to make gfxImageFrame catch errors

This will make gfxImageFrame catch the errors, but it is then up to the
decoders to properly handle the errors...

Comment 11

[Simon Fraser [no longer active]]

> I believe that QuickDraw now does support larger PixMaps, when using the
> accessor methods and opaque structures. This should give us a solution for the
> carbon build, but we are still limited on the CFM build. cc'ing some other mac
> folks.

Where did you read this? AFAIK, you're still limited to rowbytes < 0x3FFF because 
CopyBits looks at the top two bits to see if what you pass in is a BitMap, 
PixMapHandle, or CGrafPort.

Comment 12

[Brian Nesse (gone)]

I'll have to dig. I recall reading something about some "magic" they were doing
under the covers... It's been quite sometime though, and I might be thinking of
something else altogether.

Comment 13

[Brian Nesse (gone)]

It's not entirely clear to me whether this can be done without QuickTime... I do
so love sifting through Apples website for documentation.

Here's the structure that allows it:
http://developer.apple.com/techpubs/macosx/Carbon/graphics/QuickDraw/QuickDraw_Manager/DataTypes/PixMapExtension.html
The QT description documents a bit better.
http://developer.apple.com/techpubs/quicktime/qtdevdocs/APIREF/SOURCESIV/pixmapextension.htm
This is apparently attached to the PixMap structure:
http://developer.apple.com/techpubs/quicktime/qtdevdocs/APIREF/SOURCESIV/pixmap.htm

There is also an old technote that details concepts and foreshadows changes:
http://developer.apple.com/technotes/qd/qd_15.html

Comment 14

[Simon Fraser [no longer active]]

I don't see the PixMapExtension struct anywhere in Universal Headers 3.4, so it 
must have been removed. However, I do note that there is a 
'gestaltQDHasLongRowBytes'.

http://developer.apple.com/techpubs/macosx/Carbon/oss/GestaltManager/
Gestalt_Manager/Gestalts/gestaltQuickdrawFeatures.html

No docs, however.

Comment 15

[Brian Nesse (gone)]

No, I don't find it either, but at line 1727 in QuickDraw.h there is an
alternate end block for the PixMap structure which contains a "void* pmExt"
instead of the "long pmReserved". And somewhere in that documentation, I came
across a PixMapExtHandle reference to that structure... which is also not in the
headers :(

Comment 16

[Steve Dagley]

->sfraser

Comment 17

[hirata masakazu]

*** Bug 125724 has been marked as a duplicate of this bug. ***

Comment 18

[hirata masakazu]

Is this the same as bug 113406?

Comment 19

[hirata masakazu]

*** Bug 124767 has been marked as a duplicate of this bug. ***

Comment 20

[Simon Fraser [no longer active]]

This bug is distinct from bug 113406. This bug is about a *crash* in GIF
decoding code (which happens if the nsIImage hands back a null pixels pointer).
The GIF code needs fixing to better catch errors. This is pavlov's code.

Comment 21

[Terri Preston]

cc:petersen

Comment 22

[Matthias Versen [:Matti]]

*** Bug 149323 has been marked as a duplicate of this bug. ***

Comment 23

[Greg K.]

FWIW, bug 149323 contains an attached stack trace.

Comment 24

[hirata masakazu]

*** Bug 154768 has been marked as a duplicate of this bug. ***

Comment 25

[John Unruh]

WFM with the Dec 20 commercial trunk build on OSX 10.2.3.

Comment 26

[Terri Preston]

Also wfm Mac OS X trunk build 2002122608, please reopen if you still see this crash