329889 - Crash in [@ imgContainerGIF::GetFrameAt] when dragging a corrupted gif file

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Thomas Bertels]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

TB16130387E
Reproduced with Firefox 1.9a1 2006030807 and Thunderbird 1.5.

Reproducible: Always

Steps to Reproduce:
1. Drag a corrupted gif image
Actual Results:  
crash


This only happens if the file has the gif extension (tested png and jpg).

Comment 1

[Thomas Bertels]

Attachment: Text file with gif extension for the testcase

This is just a text file with the gif extension.

Comment 2

[Thomas Bertels]

Attachment: testcase

Just drag the image to crash the browser.
I forgot to mention that the "gif image" can be attached to a mail to crash the mail client.

Comment 3

[Martijn Wargers (dead)]

I'm not seeing an image (not even the broken image icon), so there isn't anything to drag for me.

From talkback ID: 
imgContainerGIF::GetFrameAt  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/modules/libpr0n/decoders/gif/imgContainerGIF.cpp, line 142]
nsTransferableFactory::Produce  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/base/src/nsContentAreaDragDrop.cpp, line 1240]
nsTransferableFactory::CreateFromEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/base/src/nsContentAreaDragDrop.cpp, line 943]
nsContentAreaDragDrop::CreateTransferable  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/base/src/nsContentAreaDragDrop.cpp, line 668]
DispatchToInterface  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 141]
nsEventListenerManager::HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 1779]
nsXULElement::HandleDOMEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/xul/content/src/nsXULElement.cpp, line 2153]
nsXULElement::HandleChromeEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/xul/content/src/nsXULElement.cpp, line 2833]
nsGlobalWindow::HandleDOMEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp, line 1585]
nsDocument::HandleDOMEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/base/src/nsDocument.cpp, line 4013]
nsGenericElement::HandleDOMEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/base/src/nsGenericElement.cpp, line 2206]
nsGenericElement::HandleDOMEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/base/src/nsGenericElement.cpp, line 2198]
nsGenericElement::HandleDOMEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/base/src/nsGenericElement.cpp, line 2198]
nsHTMLImageElement::HandleDOMEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLImageElement.cpp, line 507]
nsEventStateManager::GenerateDragGesture  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/events/src/nsEventStateManager.cpp, line 1608]
nsEventStateManager::PreHandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/events/src/nsEventStateManager.cpp, line 521]
PresShell::HandleEventInternal  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 6368]
PresShell::HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 6210]
nsViewManager::HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp, line 2559]
nsViewManager::DispatchEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp, line 2246]
HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/view/src/nsView.cpp, line 174]
nsWindow::DispatchEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1252]
nsWindow::DispatchMouseEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 5982]
ChildWindow::DispatchMouseEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 6233]
nsWindow::WindowProc  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1434]
USER32.dll + 0x3a68 (0x77d13a68)
USER32.dll + 0x3b37 (0x77d13b37)
USER32.dll + 0x3d91 (0x77d13d91)
USER32.dll + 0x3df7 (0x77d13df7)
nsAppStartup::Run  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 151]
main  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/browser/app/nsBrowserApp.cpp, line 61]
kernel32.dll + 0x214c7 (0x77e614c7)

Comment 4

[Thomas Bertels]

(In reply to comment #3)
> I'm not seeing an image (not even the broken image icon), so there isn't
> anything to drag for me.

Well, I don't know which version you're using, but anyway the image is at the top left corner of the testcase page.

Comment 5

[Martijn Wargers (dead)]

I'm using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060308 Firefox/1.6a1
and:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060118 SeaMonkey/1.5a Mnenhy/0.7.3.0
In both cases, the broken image disappears for me when the page has finished loading.

Comment 6

[Thomas Bertels]

Attachment: testcase

You're right, I didn't notice it.
I fixed the width and height to make it visible.

Comment 7

[Bengt-Erik Soderstrom]

I can confirm the crash also in a linux build:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060308 SeaMonkey/1.5a

BE

Comment 8

[Martijn Wargers (dead)]

Attachment: testcase2, self-contained

Ok, this regressed between 2004-09-13 and 2004-09-16:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-09-13+07&maxdate=2004-09-16+09&cvsroot=%2Fcvsroot

Maybe because of bug 258121?

Comment 9

[Martijn Wargers (dead)]

In comment 3, I mentioned I could not see the image with the testcase, using current trunk build. Actually that's a regression, I filed bug 329896 for that.

Comment 10

[Martijn Wargers (dead)]

No, backing out the patch for bug 258121 doesn't help.

Comment 11

[Martijn Wargers (dead)]

Ria got this range, between 2004-09-14 and 2004-09-15:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-09-14+08+&maxdate=2004-09-15+07&cvsroot=%2Fcvsroot

Comment 12

[Martijn Wargers (dead)]

Attachment: backtrace from debug build

So this seems to be a regression from bug 96108, caused by the check-in of https://bugzilla.mozilla.org/attachment.cgi?id=158824.

First I get an assertion:
#1  0x6ff47483 in Break(char const*) (
    aMsg=0x22d8b8 "###!!! ASSERTION: index out of range: '0 <= aIndex && aIndex
< Count()', file ../../../dist/include/xpcom/nsVoidArray.h, line 71")
    at c:/mozilla/mozilla/xpcom/base/nsDebugImpl.cpp:434
#2  0x6ff471e0 in NS_DebugBreak_P (aSeverity=1,
    aStr=0x65d09fb0 "index out of range",
    aExpr=0x65d09f90 "0 <= aIndex && aIndex < Count()",
    aFile=0x65d09f50 "../../../dist/include/xpcom/nsVoidArray.h", aLine=71)
    at c:/mozilla/mozilla/xpcom/base/nsDebugImpl.cpp:351
#3  0x65d563f0 in nsVoidArray::FastElementAt(int) const (this=0xee08b24,
    aIndex=0) at ../../../../dist/include/xpcom/nsVoidArray.h:71

The the crash:
#0  0x65d563f8 in nsVoidArray::FastElementAt(int) const (this=0xee08b24,
    aIndex=0) at ../../../../dist/include/xpcom/nsVoidArray.h:72
#1  0x65d565b4 in nsCOMArray_base::ObjectAt(int) const (this=0xee08b24,
    aIndex=0) at ../../../../dist/include/xpcom/nsCOMArray.h:100
#2  0x65d56378 in nsCOMArray<gfxIImageFrame>::ObjectAt(int) const (
    this=0xee08b24, aIndex=0)
    at ../../../../dist/include/xpcom/nsCOMArray.h:160
#3  0x65d56394 in nsCOMArray<gfxIImageFrame>::operator[](int) const (
    this=0xee08b24, aIndex=0)
    at ../../../../dist/include/xpcom/nsCOMArray.h:170

Comment 13

[Jesse Ruderman]

Hitting that assertion in FastElementAt is a bad sign.

Comment 14

[Thomas Bertels]

e.g. corrupted gif file ?
This bug hasn't been reproduced with other files extensions (see the bug details).

Comment 15

[Jesse Ruderman]

Oops, I didn't mean to make that change to the summary.  Reverting now.

Comment 16

[Darin Fisher]

Attachment: v1 patch

simple patch.  protect against calls to GetFrameAt that would dereference beyond the end of the array.

Comment 17

[chris hofmann]

pav, can you review?

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 219693 [details] [diff] [review]
v1 patch

sr=dveditz
trying a different reviewer

Comment 19

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

Comment on attachment 219693 [details] [diff] [review]
v1 patch

I don't think pav would appreciate me reviewing imagelib patches

but, why does the caller pass an out-of-range index here?

Comment 20

[Stuart Parmenter]

yeah, i'd like to know the same -- we shouldn't ever hit this case and should probably protect it somewhere else.

Comment 21

[Darin Fisher]

> yeah, i'd like to know the same -- we shouldn't ever hit this case and should
> probably protect it somewhere else.

Probably true, but this is a scriptable interface.  The general rule is that any scriptable interface must protect against out-of-bound input errors (including null pointers).  We try to make sure that Javascript extensions cannot crash the app.

Comment 22

[Darin Fisher]

Note: I'm converting a crash into a warning.  We can still track down the cause of the warning.  Since there is a code-freeze coming up on Monday for 1.5.0.4, I'd really like to see this crash fixed in that release.  I won't have a chance to investigate this crash further before then.

Comment 23

[Daniel Veditz [:dveditz]]

Comment on attachment 219693 [details] [diff] [review]
v1 patch

Pav: please green-light this band-aide for the 1.8.0.4 release. Or propose an alternative.

Comment 24

[Stuart Parmenter]

Comment on attachment 219693 [details] [diff] [review]
v1 patch

r/approval+ -- leave this open and someone find a real fix please.

Comment 25

[Daniel Veditz [:dveditz]]

Comment on attachment 219693 [details] [diff] [review]
v1 patch

approved for 1.8.0 branch, a=dveditz for drivers

Comment 26

[Darin Fisher]

fixed-on-trunk, fixed1.8.1, fixed1.8.0.4

Comment 27

[Jesse Ruderman]

Filed followup bug 329889: "Dragging broken GIF shouldn't call imgContainerGIF::GetFrameAt(0)".

Comment 28

[Jesse Ruderman]

Sorry, bug 336295.

Comment 29

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on SeaMonkey trunk / Windows XP using build 2006-05-03-10.

Comment 30

[Darin Fisher]

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

Comment 31

[Jay Patel [:jay]]

v.fixed on 1.8.0 branch: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4, no crash with either testcase, and no incidents in current Firefox15 Talkback data.