Closed Bug 403578 Opened 12 years ago Closed 12 years ago
Memory corruption in GIF decoder due to tpixel outside of colormap
13.75 KB, text/plain
11.07 KB, text/plain
98.38 KB, image/gif
1.96 KB, patch
|Details | Diff | Splinter Review|
blaming thebes based on the first part of the backtrace. ======= Backtrace: ========= /lib/libc.so.6[0xe5c583] /lib/libc.so.6(__libc_malloc+0x7e)[0xe5debe] /usr/lib/libstdc++.so.6(_Znwj+0x27)[0x4cb0547] /work/mozilla/builds/1.9.0/mozilla/firefox-debug/dist/bin/components/libgkgfxthebes.so[0x1a10113]
Igor, I found this looking at crash-reports for js related stacks. maybe this is something you could look at?
AFAIK, this just means that a malloc() done by thebes triggered a heap check which discovered the corruption. Doesn't mean that thebes is responsible for it. If it's reproducible with the given testcase, should be easy to figure out the actual problem using valgrind/purify/etc.
I'm thinking ImageLib here. Maybe the same issue as bug 403580
over to imagegfx.
Component: GFX: Thebes → Image: GFX
QA Contact: thebes → image.gfx
bug 143046 looks like the culprit.
Bug 4033363 has covered this 'imgContainer:DrawFrameTo' problem.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 403363
And confirmed that with my patch in 403363 the URL above (http://omnimaga.org/index.php?showtopic=1548) doesn't crash anymore.
Actually, the crash is indeed somewhere in nsGIFDecoder2.cpp, on the line with: mColormap[mGIFStruct.tpixel] = 0; The offending gif image seems to have a tpixel outside the colormap (globa_colormap_depth=3 (so 8 colors), and tpixel =15.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
The image that causes the crash of http://img249.imageshack.us/img249/7711/ptiani1fm3.gif
Content induced crash - so marking blocking and making a p2.
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
The crashing image has tpixel 15 and depth 3, and other image viewers (such as IrfanView and Opera) report this image has 4 bpp. So, for this image we need to change the depth to accomodiate for the tpixel value. This didn't crash before as we always used colormaps with 256 entries, but with the patch from bug 143046, palettes are sized to the actual depth. So, to fix this issue, we increase the depth value till tpixel also fits in the colormap. (note: the other solution would be to ignore tpixel values outside the depth range, but then this image still doesn't display correctly)
Attachment #289291 - Flags: review?
Comment on attachment 289291 [details] [diff] [review] Patch to fix depth for images with tpixel outside the depth range (clearing a?, it's a blocker, doesn't need approval, please land)
Checking in nsGIFDecoder2.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp,v <-- nsGIFDec oder2.cpp new revision: 1.88; previous revision: 1.87 done Checked in. This regressed between 2007-11-07 and 2007-11-08, when bug 143046 was fixed, so a regression from that bug. So since this is a recent regression, can this bug be opened up?
Summary: glibc detected firefox-bin: malloc(): memory corruption → Memory corruption in GIF decoder due to tpixel outside of colormap
Followup bug to address the regression is bug 408310.
Whiteboard: [sg:critical?] post 1.8-branch
You need to log in before you can comment on or make changes to this bug.