137128 - an animated gif that crashes mozilla

Status: RESOLVED FIXED

(Core Graveyard :: Image: Painting, defect)

Description

[Daniel Resare]

I found an animated gif that crashes mozilla, reproduced on i386/Linux with the
nightly build from 20020411. The gif can be found on the url referenced above.

Comment 1

[Olivier Cahagne]

wfm using build 2002041203 on Win2k. Might be Linux only.
Reporter, can you post Talkback ID by running
'mozilla/components/talkback/talkback' ?

Comment 2

[Vadim Berezniker]

#0  0x4062b4bc in memcpy () from /lib/libc.so.6
#1  0x40eea6d4 in nsImageGTK::DrawToImage(nsIImage*, int, int, int, int)
(this=0x8896608, aDstImage=0x8775f70, aDX=122, 
    aDY=20, aDWidth=57, aDHeight=12) at nsImageGTK.cpp:1978
#2  0x41afd93a in gfxImageFrame::DrawTo(gfxIImageFrame*, int, int, int, int)
(this=0x8211db8, aDst=0x889df28, aDX=122, 
    aDY=20, aDWidth=57, aDHeight=12) at gfxImageFrame.cpp:394
#3  0x418892dd in imgContainer::DoComposite(gfxIImageFrame**, nsRect*, int, int)
(this=0x86aceb0, aFrameToUse=0xbffff2c0, 
    aDirtyRect=0xbffff2d0, aPrevFrame=1, aNextFrame=2) at imgContainer.cpp:516
#4  0x41888f13 in imgContainer::Notify(nsITimer*) (this=0x86aceb0,
timer=0x8211d78) at imgContainer.cpp:455
#5  0x4023fddc in nsTimerImpl::Process() (this=0x8211d78) at nsTimerImpl.cpp:332
#6  0x4023ff10 in handleMyEvent(MyEventType*) (event=0x41b58a48) at
nsTimerImpl.cpp:379
#7  0x40237d46 in PL_HandleEvent (self=0x41b58a48) at plevent.c:596
#8  0x40237b59 in PL_ProcessPendingEvents (self=0x80fa5a0) at plevent.c:526
#9  0x4023a1be in nsEventQueueImpl::ProcessPendingEvents() (this=0x80fa558) at
nsEventQueue.cpp:388
#10 0x40b5acd4 in event_processor_callback(void*, int, GdkInputCondition)
(data=0x80fa558, source=6, 
    condition=GDK_INPUT_READ) at nsAppShell.cpp:184
#11 0x40b5a740 in our_gdk_io_invoke(_GIOChannel*, GIOCondition, void*)
(source=0x41b019b0, condition=G_IO_IN, 
    data=0x41b019a0) at nsAppShell.cpp:76
#12 0x4048db71 in g_io_channel_unix_get_fd () from /usr/lib/libglib-1.2.so.0
#13 0x4048eef8 in g_source_remove_by_funcs_user_data () from
/usr/lib/libglib-1.2.so.0
#14 0x4048f543 in g_source_remove_by_funcs_user_data () from
/usr/lib/libglib-1.2.so.0
#15 0x4048fe7c in g_main_run () from /usr/lib/libglib-1.2.so.0
#16 0x403d8139 in gtk_main () from /usr/lib/libgtk-1.2.so.0
#17 0x40b5b305 in nsAppShell::Run() (this=0x8142378) at nsAppShell.cpp:364
#18 0x40af864e in nsAppShellService::Run() (this=0x8129578) at
nsAppShellService.cpp:308
#19 0x0805d0cd in main1(int, char**, nsISupports*) (argc=1, argv=0xbffff874,
nativeApp=0x0) at nsAppRunner.cpp:1414
#20 0x0805dd2a in main (argc=1, argv=0xbffff874) at nsAppRunner.cpp:1762
#21 0x405c0283 in __libc_start_main () from /lib/libc.so.6

Comment 3

[Daniel Resare]

Talkback id is TB123655H

Comment 4

[Matthias Versen [:Matti]]

confirming based on Vadim Berezniker's stack

Comment 5

[Andrew Schultz]

debug build throws assertions before crashing:

###!!! ASSERTION: Received an EndFrameDecode call with an invalid frame number:
'currentFrame', file imgContainer.cpp, line 230

Comment 6

[ArronM (:paper)]

Cause: Although the image is defined as 54x55, many of the frames are offset to
well beyond 54 (x).  For example, Frame 3 starts at x_offset of 155.

Comment 7

[ArronM (:paper)]

Attachment: Patch to fix x_offset + frame width > screen_width

Comment 8

[Andrew Schultz]

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

Comment 9

[Matthias Versen [:Matti]]

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

Comment 10

[Pedro Madeira]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc3) Gecko/20020523

Happens the same to me with the latest build

Comment 11

[moz]

Does not crash on my 1.00RC2 in Windows 2000

Comment 12

[ArronM (:paper)]

Attachment: return if trying to draw beyond bounds of dstImage

I don't know what I was thinking when I created that first patch.

This one is right.  Really :P

What was happening was ValidWidth was being set to negative because what was
being passed in was a position (aDX) beyond the width of aDstImage.  As such:
  PRInt32 ValidWidth = ( aDWidth < ( dest->mWidth - aDX ) ) ? aDWidth : (
dest->mWidth - aDX ); 

In the crasher image, one frame started at 125 (aDX) and DrawToImage was being
called to draw that frame into the mCompositingFrame.  mCompositingFrame is
only 54 in width.  It was set to 54 because the main gif header said the image
was only 54 wide (it lied).  54 - 125 = (some neg. number).  Later, memcpy was
called for each row to copy (ValidWidth) bytes of the frame to
mCompositingFrame.  And Boom!

This patch fixes the problem by exiting if caller is requesting to draw beyond
the bounds of the dstImage

Comment 13

[Matthias Versen [:Matti]]

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

Comment 14

[Doug Turner (:dougt)]

gtk

Comment 15

[Olivier Cahagne]

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

Comment 16

[Matthias Versen [:Matti]]

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

Comment 17

[Sebastian Biallas]

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

Comment 18

[tor]

Comment on attachment 85028 [details] [diff] [review]
return if trying to draw beyond bounds of dstImage

sr=tor

Comment 19

[ArronM (:paper)]

Attachment: including XLib in patch

Lib has the same problem.

Comment 20

[Roland Mainz]

Comment on attachment 89621 [details] [diff] [review]
including XLib in patch

r=[e-mail]

Comment 21

[tor]

Comment on attachment 89621 [details] [diff] [review]
including XLib in patch

sr=tor

Comment 22

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

fixed

Comment 23

[ArronM (:paper)]

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