Closed Bug 424333 Opened 13 years ago Closed 12 years ago

crafted gif file will crash firefox [XError: 'BadAlloc (insufficient resources for operation)']

Categories

(Core :: Graphics, defect)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: hanno, Assigned: vlad)

References

(Blocks 1 open bug, )

Details

(Keywords: crash, platform-parity)

Attachments

(5 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.1.12) Gecko/20080212 Firefox/2.0.0.12
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.1.12) Gecko/20080212 Firefox/2.0.0.12

Sam Hovecar created the fuzzing tool zzuf and published a number of samples on it's page:
http://sam.zoy.org/zzuf/

the lol-firefox.gif file still crashes current firefox on my system.

Reproducible: Always
Attached image gif that crashes ff
Doesn't crash for me with SM trunk (1 day old) or Firefox 3b4.

Reporter: Could you try it again with a Firefox3 Beta build ?
(you can install it in a different directory )
3.0b4 gives the same result:

hanno@laverne /tmp/firefox $ ./firefox --sync
The program 'firefox-bin' received an X Window System error.
This probably reflects a bug in the program.
The error was 'BadAlloc (insufficient resources for operation)'.
  (Details: serial 66009 error_code 11 request_code 53 minor_code 0)
  (Note to programmers: normally, X errors are reported asynchronously;
   that is, you will receive the error a while after causing it.
   To debug your program, run it with the --sync command line
   option to change this behavior. You can then get a meaningful
   backtrace from your debugger if you break on the gdk_x_error() function.)
Component: General → ImageLib
Keywords: crash
Product: Firefox → Core
QA Contact: general → imagelib
Version: unspecified → Trunk
This fixes my branch debug build...
I'm applying the limit we have in nsImageGTK::Init():
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/src/gtk/nsImageGTK.cpp&rev=1.152&root=/cvsroot&mark=181-184#175
to every gdk_pixmap_new() call site.
Status: UNCONFIRMED → NEW
Component: ImageLib → GFX
Ever confirmed: true
Keywords: pp
QA Contact: imagelib → general
Summary: crafted gif file will crash firefox → crafted gif file will crash firefox [XError: 'BadAlloc (insufficient resources for operation)']
Attached patch Trunk patch, rev. 1 (obsolete) — Splinter Review
Assignee: nobody → mats.palmgren
Status: NEW → ASSIGNED
Attachment #311249 - Flags: superreview?(vladimir)
Attachment #311249 - Flags: review?(vladimir)
Hrm; we probably should fix cairo, but there should already be code inside Thebes to limit image size.. I can take a look and see why that's not being hit.  In the meantime, I'll poke cworth and point them at this and see what they think about doing this in Cairo proper.
After applying the "Trunk patch, rev. 1" I still crash after ~5 seconds when loading the gif (attachment 310964 [details].)
(In reply to comment #7)
> After applying the "Trunk patch, rev. 1" I still crash after ~5 seconds when
> loading the gif (attachment 310964 [details].)

Cancel that -- it does seem to fix the crash after all.  I think I was running the wrong build when it crashed before.  Sorry about that.
Attached patch Branch 1.9.0 patch, rev. 2 (obsolete) — Splinter Review
Same as rev. 1 with an addition to fix bug 427385:
There are two calls to gdk_pixmap_new() in gfx/thebes/src/gfxPlatformGtk.cpp
that needs to be fixed in a similar way.
Attachment #311249 - Attachment is obsolete: true
Attachment #313977 - Flags: superreview?(vladimir)
Attachment #313977 - Flags: review?(vladimir)
Attachment #311249 - Flags: superreview?(vladimir)
Attachment #311249 - Flags: review?(vladimir)
Blocks: 427385
Requesting blocking1.9 since there is a low risk patch and with every
crash there is potential dataloss for the user.
Flags: blocking1.9?
There's a fix in progress for this on the cairo list, but wouldn't block 1.9 for it.  However, I think we can avoid the bug on our end with the fixes in gfxXlibSurface and gfxPlatformGtk; I'll look at those shortly.
Flags: wanted1.9.0.x+
Flags: blocking1.9?
Flags: blocking1.9-
Blocks: 428258
Blocks: 429752
I tested attachment #313977 [details] [diff] [review] with some minor updates (this attachment).
With it, I noticed:

- Firefox doesn't crash anymore :-)
- The image region is not (re-)drawn :-(

I think we need some works for this bug.
(In reply to comment #12)
> - The image region is not (re-)drawn :-(

Oops! Sorry, the redraw problem is for bug 427385 (on PNG images). On GIF images, it doesn't happened.
Blocks: 441210
Attached patch Updated for mozilla-central (obsolete) — Splinter Review
Attachment #313977 - Attachment description: Trunk patch, rev. 2 → Branch 1.9.0 patch, rev. 2
Attachment #326232 - Flags: superreview?(vladimir)
Attachment #326232 - Flags: review?(vladimir)
vlad did this get fixed upstream?
Why no tests for this?
Blocks: 444121
Attached patch Updated for mozilla-central (obsolete) — Splinter Review
Attachment #326232 - Attachment is obsolete: true
Attachment #331580 - Flags: superreview?(vladimir)
Attachment #331580 - Flags: review?(vladimir)
Attachment #326232 - Flags: superreview?(vladimir)
Attachment #326232 - Flags: review?(vladimir)
(In reply to comment #12)
> Created an attachment (id=318131) [details]
> Updated trunk patch, rev. 2
> 
> I tested attachment #313977 [details] [diff] [review] with some minor updates (this attachment).
> With it, I noticed:
> 
> - Firefox doesn't crash anymore :-)
> - The image region is not (re-)drawn :-(

This is interesting.
XCreatePixmap fail -> cario_surface_create_in_error (NO_MEMORY)  (cario_surface_create_in_error doesn't have something like CAIRO_INT_STATUS_UNSUPPORTED) ->xlib backend composite fail as NO_MEMORY

The result is _cairo_surface_composite won't fallback to _cairo_surface_fallback_composite for NO_MEMORY.

I noticed some websites have a huge gif as background. That would be a big problem for Fx 3 on Linux and Solaris.
I hope we can get the fix into 1.9.0 branch.
vlad,

your commit of
http://gitweb.freedesktop.org/?p=cairo;a=commitdiff;h=2cf82eaf0d08e68b787bb0792da97e73d8d4ce38
solved the issue with no redraw problem.

But your other commit
http://gitweb.freedesktop.org/?p=cairo;a=commitdiff;h=d756a4d6323d23cecb928822cdac7528859e7cf3
totally broke the drawing.

I don't know how to reproduce the "UNSUPPORTED loop", you're trying to fix.

If you want to reproduce the drawing issue, apply your 2 patches onto mozilla-central, and change XLIB_COORD_MAX to a very small value, say 80.
Then you can reproduce it with http://www.mozilla.org.
BTW: the second commit's comment doesn't match the code
the comment is "[xlib] check for too-large surface size in create similar to avoid UNSUPPORTED loop"
but the code change is for _cairo_xlib_surface_clone_similar.

If NO_MEM is returned here, it won't fallback to image surface or _cairo_surface_fallback_clone_similar.
Hm, what do you mean by broke the drawing?  If nothing is getting drawn, that's intentional; there isn't anything we can really do if we get to that point.  However, we should be (even before) trying a smaller interest rectangle from the image.

The problem is that if xlib_surface_create_similar_with_format returns NULL and thus clone returns UNSUPPORTED, in some cases we'll end up going through a fallback path and eventually getting back to calling clone_similar again... thus leading to an infinite loop and eventually running out of stack.  (It can be reproduced with the cairo large-source.c test.)

I'll test though and see what's happening in the actual calling path that mozilla's calling.
Yes, nothing is getting drawn. But I can't agree it is our intention.
The browser window is not over 32767 x 32767 (even it is we could have a way to handle it.), so we can always fallback to image surface safely.
The gif is displayed well on Mac, and with Firefox 2 on Linux. Why not Firefox 3 with cairo?

We got different results because we're using different test case. 
If _cairo_surface_create_similar_scratch() got a large width or height,
other->backend->create_similar (other, content, width, height); will fail (other->backend is XLIB)
then it fallbacks to image surface, and everything is fine.
But if the width, height is small, but the pattern is huge, _cairo_surface_create_similar_scratch() still returns xlib surface,
then it will be a infinite loop between _cairo_surface_fallback_clone_similar and _cairo_paint.

It's fixable. But I don't have a reasonable patch yet.
Flags: blocking1.9.0.3?
Flags: blocking1.9.0.3? → blocking1.9.0.3+
Flags: blocking1.9.0.4+ → blocking1.9.0.5+
Any update here? It's been over a month since the last comment...
Here's the Gecko side of this patch.  Avoids the BadAlloc crash for this and similar bugs.

Doesn't do anything for actual display of the large images, but that should be a separate bug.
Attachment #344529 - Flags: review?(pavlov)
Assignee: mats.palmgren → vladimir
Comment on attachment 344529 [details] [diff] [review]
add checks to gecko side as well

Switching r? over to joe
Attachment #344529 - Flags: review?(pavlov) → review?(joe)
Attachment #344529 - Flags: review?(joe) → review+
Gecko-side patches checked in.  With the recent changes in the cairo version that's been upgraded, we -might- now get correct rendering of large images under X.  We should probably add a reftest for this.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [needs 1.9.0 patch? wontfix for 1.9.0?]
We're not going to take this fix on the 1.9.0 branch due to risk.
Flags: blocking1.9.0.5+ → blocking1.9.0.5-
Attachment #313977 - Attachment is obsolete: true
Attachment #313977 - Flags: superreview?(vladimir)
Attachment #313977 - Flags: review?(vladimir)
Attachment #331580 - Attachment is obsolete: true
Attachment #331580 - Flags: superreview?(vladimir)
Attachment #331580 - Flags: review?(vladimir)
Duplicate of this bug: 474305
Product: Core → Core Graveyard
Component: GFX → GFX: Thebes
Product: Core Graveyard → Core
QA Contact: general → thebes
Duplicate of this bug: 474220
No longer blocks: 428258
Duplicate of this bug: 428258
No longer blocks: 429752
Duplicate of this bug: 429752
Duplicate of this bug: 480783
Duplicate of this bug: 484558
Duplicate of this bug: CVE-2009-2044
FWIW, this is the cairo patch
http://cgit.freedesktop.org/cairo/commit/?id=2cf82eaf0d08e68b787bb0792da97e73d8d4ce38
plus attachment 344529 [details] [diff] [review] in this bug, plus a check that 'solid_picture'
is valid, backported to 1.9.0.  It seems to fix this bug and bug 390768
on the 1.9.0 branch.
Blocks: 390768
Duplicate of this bug: 390768
Duplicate of this bug: 455701
Duplicate of this bug: 427385
is the patch in 1.9.0? If so, should whiteboard say wontfix ?
We're not going to take this on 1.9.0 per comment 29.
Flags: wanted1.9.0.x+ → wanted1.9.0.x-
Whiteboard: [needs 1.9.0 patch? wontfix for 1.9.0?]
Hi,

Sorry to revive this bug but, using Firefox 3.5.5, it just happened on my Fedora 11 system.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091105 Fedora/3.5.5-1.fc11 Firefox/3.5.5

$ uname -r
2.6.30.9-96.fc11.i586

The image that froze my system was this 21600 x 21600 JPEG (11.5MB) http://veimages.gsfc.nasa.gov/7103/world.topo.bathy.200404.3x21600x21600.C2.jpg 
Got the image from this site: http://visibleearth.nasa.gov/view_rec.php?id=7103

On a Windows Vista system, it seems that Firefox refuses to display the image complaining that it is invalid or corrupt.

Please look it up.

Thanks.
Works fine on Fedora 12/firefox-3.5.6. I've got a message about broken image.
Martin, I confirm your result on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.6) Gecko/20091216 Fedora/3.5.6-1.fc11 Firefox/3.5.6

It seems that the latest FFox updates fixed the issue.

Thanks everybody!
You need to log in before you can comment on or make changes to this bug.