Closed Bug 329889 Opened 15 years ago Closed 15 years ago

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

Categories

(Core :: ImageLib, defect)

x86
All
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: tbertels+bugzilla, Assigned: darin.moz)

References

Details

(5 keywords)

Crash Data

Attachments

(5 files, 1 obsolete file)

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).
This is just a text file with the gif extension.
Attached file testcase (obsolete) —
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.
Keywords: crash, testcase
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)
(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.
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.
Attached file testcase
You're right, I didn't notice it.
I fixed the width and height to make it visible.
Attachment #214541 - Attachment is obsolete: true
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
OS: Windows XP → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
No, backing out the patch for bug 258121 doesn't help.
Summary: Crash when dragging a corrupted gif file → Crash in [@ imgContainerGIF::GetFrameAt] when dragging a corrupted gif file
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
Blocks: 96108
Component: Drag and Drop → XPCOM
Flags: blocking1.9a1?
Keywords: regression
Hitting that assertion in FastElementAt is a bad sign.
Flags: blocking1.8.0.3?
Assignee: nobody → pavlov
Component: XPCOM → ImageLib
Summary: Crash in [@ imgContainerGIF::GetFrameAt] when dragging a corrupted gif file → Crash in [@ imgContainerGIF::GetFrameAt] when dragging broken image (e.g. corrupted gif file)
e.g. corrupted gif file ?
This bug hasn't been reproduced with other files extensions (see the bug details).
Oops, I didn't mean to make that change to the summary.  Reverting now.
Summary: Crash in [@ imgContainerGIF::GetFrameAt] when dragging broken image (e.g. corrupted gif file) → Crash in [@ imgContainerGIF::GetFrameAt] when dragging a corrupted gif file
Flags: blocking1.8.0.3? → blocking1.8.0.3+
Attached patch v1 patchSplinter Review
simple patch.  protect against calls to GetFrameAt that would dereference beyond the end of the array.
Assignee: pavlov → darin
Status: NEW → ASSIGNED
Attachment #219693 - Flags: review?(pavlov)
pav, can you review?
Comment on attachment 219693 [details] [diff] [review]
v1 patch

sr=dveditz
trying a different reviewer
Attachment #219693 - Flags: superreview+
Attachment #219693 - Flags: review?(pavlov)
Attachment #219693 - Flags: review?(cbiesinger)
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?
Attachment #219693 - Flags: review?(cbiesinger) → review?(pavlov)
yeah, i'd like to know the same -- we shouldn't ever hit this case and should probably protect it somewhere else.
> 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.
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.
Whiteboard: needs r=pavlov, a1.8.1=pavlov
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.
Attachment #219693 - Flags: approval-branch-1.8.1?(pavlov)
Comment on attachment 219693 [details] [diff] [review]
v1 patch

r/approval+ -- leave this open and someone find a real fix please.
Attachment #219693 - Flags: review?(pavlov)
Attachment #219693 - Flags: review+
Attachment #219693 - Flags: approval-branch-1.8.1?(pavlov)
Attachment #219693 - Flags: approval-branch-1.8.1+
Whiteboard: needs r=pavlov, a1.8.1=pavlov → needs branch landing
Comment on attachment 219693 [details] [diff] [review]
v1 patch

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #219693 - Flags: approval1.8.0.4+
fixed-on-trunk, fixed1.8.1, fixed1.8.0.4
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Filed followup bug 329889: "Dragging broken GIF shouldn't call imgContainerGIF::GetFrameAt(0)".
Sorry, bug 336295.
No longer depends on: 336388
Depends on: 336432
No longer depends on: 336432
Verified FIXED on SeaMonkey trunk / Windows XP using build 2006-05-03-10.
Status: RESOLVED → VERIFIED
*** Bug 320685 has been marked as a duplicate of this bug. ***
Flags: blocking1.9a1?
Whiteboard: needs branch landing
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.
Crash Signature: [@ imgContainerGIF::GetFrameAt]
You need to log in before you can comment on or make changes to this bug.