164381 - invalid read in nsImageGTK::UpdateCachedImage()

Status: VERIFIED DUPLICATE of bug 174259

(Core Graveyard :: Image: Painting, defect)

Description

[Andrew Schultz]

while testing bug 163874 (loading www.xoip.com), valgrind complains a lot as
follows:
Invalid read of size 1
 at 0x4697DA56: nsImageGTK::UpdateCachedImage(void) (nsImageGTK.cpp:438)
 by 0x4697F366: nsImageGTK::Optimize(nsIDeviceContext *) (nsImageGTK.cpp:1742)
 by 0x510B990E: gfxImageFrame::SetMutable(int) (gfxImageFrame.cpp:150)
 by 0x4707E458: imgContainer::DecodingComplete(void) (imgContainer.cpp:251)
Address 0x41E9B8F0 is 0 bytes after a block of size 40 alloc'd
 at 0x400429EC: __builtin_vec_new (vg_clientfuncs.c:152)
 by 0x4697D5FB: nsImageGTK::Init(int, int, int, nsMaskRequirements)
(nsImageGTK.cpp:224)
 by 0x510B9822: gfxImageFrame::Init(int, int, int, int, int) (gfxImageFrame.cpp:120)
 by 0x4C9845F6: HaveDecodedRow(void *, unsigned char *, int, int, int, int,
unsigned char, int) (nsGIFDecoder2.cpp:420)

it also complains about invalid reads 1-5 bytes after the same block.

It looks like the worst that could happen here is that mIsSpacer would be set to
the wrong value.

The fun part is to now figure out which gif is at fault.

Comment 1

[Andrew Schultz]

Attachment: testcase

this testcase should trigger the invalid reads.  Probably only need one of the
"spacers", but I got tired of waiting for valgrind.

Comment 2

[Andrew Schultz]

The image (spacer.gif) is 10x10.  mAlphaRowBytes is 4.  nsImageGTK (line 224)
allocates mAlphaRowBytes x aHeight (4x10=40) bytes for mAlphaBits.

UpdateCacheImage (line 437) then traverses the width and height looking for
non-zero elements.  The structure makes it look like it thinks that
(mAlphaRowBytes == right-left), but that is incorrect.

Conceivably, this results in mIsSpacer always being PR_FALSE even when it should
be PR_TRUE.

Comment 3

[Andrew Schultz]

*** This bug has been marked as a duplicate of 174259 ***

Comment 4

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

indeed a dup.