I recently ran firefox (trunk) under purify and I saw this (starting Firefox, loading the default start page http://www.mozilla.org/projects/minefield/): [E] IPW: Invalid pointer write in nsGIFDecoder2::OutputRow(void) {1 occurrence} Writing 4 bytes to 0x0d692ffc (1 byte at 0x0d692fff illegal) Address 0x0d692ffc points into a committed VirtualAlloc'd block Thread ID: 0xecc Error location nsGIFDecoder2::OutputRow(void) [e:\tip\mozilla\modules\libpr0n\decoders\gif\nsgifdecoder2.cpp:458] PRUint32 *to = ((PRUint32*)rowp) + mGIFStruct.width; PRUint32 *cmap = mColormap; for (PRUint32 c = mGIFStruct.width; c > 0; c--) { => *--to = cmap[*--from]; } // Duplicate rows nsGIFDecoder2::DoLzw(BYTE const*) [e:\tip\mozilla\modules\libpr0n\decoders\gif\nsgifdecoder2.cpp:617] nsGIFDecoder2::GifWrite(BYTE const*,UINT) [e:\tip\mozilla\modules\libpr0n\decoders\gif\nsgifdecoder2.cpp:715] nsGIFDecoder2::ProcessData(BYTE *,UINT,UINT *) [e:\tip\mozilla\modules\libpr0n\decoders\gif\nsgifdecoder2.cpp:242] ReadDataOut [e:\tip\mozilla\modules\libpr0n\decoders\gif\nsgifdecoder2.cpp:187] nsGIFDecoder2::WriteFrom(nsIInputStream *,UINT,UINT *) [e:\tip\mozilla\modules\libpr0n\decoders\gif\nsgifdecoder2.cpp:261] imgRequest::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream *,UINT,UINT) [e:\tip\mozilla\modules\libpr0n\src\imgrequest.cpp:872] ProxyListener::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream *,UINT,UINT) [e:\tip\mozilla\modules\libpr0n\src\imgloader.cpp:877] nsHttpChannel::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream *,UINT,UINT) [e:\tip\mozilla\netwerk\protocol\http\src\nshttpchannel.cpp:4497] nsInputStreamPump::OnStateTransfer(void) [e:\tip\mozilla\netwerk\base\src\nsinputstreampump.cpp:508] No idea if this is a real problem or not, or if it's potentially exploitable etc. Marking security sensitive just in case.

This code in nsGIFDecoder2::OutputRow was added in bug 143046.

The interesting aspect is that the problem seems to be in the last byte of the mImageData buffer: Writing 4 bytes to 0x0d692ffc (1 byte at 0x0d692fff illegal) Address 0x0d692ffc points into a committed VirtualAlloc'd block 0x0f692ffc points to an even/dword align position, but 0x0f692fff (the 1 byte that Purify is complaining about) is the last byte of a dword. So, how come that this block doesn't end 'dword' aligned? As this memory block is allocated through gfxImageFrame/nsIIMage/gfxImageSurface (http://lxr.mozilla.org/seamonkey/source/gfx/thebes/src/gfxImageSurface.cpp#44), it is really strange how this block could end up not DWORD padded... And for the 'security-sensitive' flag, there is no way that one could misuse this one byte issue.

Marking blocking just so we can investigate and confirm this isn't a serious issue...

There are also crashes in imgContainer::DrawFrameTo. Currently nr 15 on the list of topcrashers: http://crash-stats.mozilla.com/topcrasher/bybranch/1.9 http://crash-stats.mozilla.com/report/list?range_unit=weeks&branch=1.9&range_value=2&signature=imgContainer%3A%3ADrawFrameTo%28gfxIImageFrame%2A%2C+gfxIImageFrame%2A%2C+nsRect%26%29 Is that related to this bug?

Sorry, never mind, that's bug 403363.

The report from Purify is really strange, and I don't know what is happening there. In short Purify is reporting that the last byte is 'illegal', but the first 3 bytes at 0xd592ffc are legal. Considering that all allocations are DWORD aligned, this can never happen...

Purify still keeps track of the exact allocation size -- so even though the app might not crash nor scribble over memory, this could be indicating that something is allocating a 1-byte too small buffer.

Not sure what is going on here.

There is something 'odd' here. The image data allocated is always in units of PRUint32, allocations are always on 4-byte boundary. So, this is really strange...

So, in short, as long there is no more information on this, this will be very difficult to analyse/solve.

This bug doesn't appear to be going anywhere. Does it need to remain hidden or can we remove that flag per comment 2.

I suppose it could be a Purify bug/false positive. Interesting memory address. Looks like its writing to the very end of a page boundary so its also possible we're hitting some sort of corner case with Purify array guards? Marking sg:investigate for now since I suspect this will take significant research to figure out one way or another. Though I wouldn't assume that one byte overwrites are inherently immune from abuse. There have been many off-by-one exploits.

Does this have any relation to bug 514776? If one runs purify with a current trunk build, is this same IPW still showing up?

We should test this under Valgrind and ASan to see if this is real or not. Does Purify even still run on our codebase?

Josh retested GIFs with Valgrind and decoder with ASan and neither as seeing this problem.

It may be WORKSFORME now, but that doesn't mean it isn't still there. I should note that there was no indication that this happened on *all* GIFs, but that it happened on one particular GIF - which is not an unusual form for such errors in image decoders; they're often data-dependent. Also note that http://www.mozilla.org/projects/minefield/ has certainly changed in the last 5 years, so going to that URL isn't a good test (if you can get the state of that URL on the day/month or even year it was reported, that'd be a better test. There's no indication in any of the comments if anyone was able to reproduce it or not; perhaps no one tried since Purify isn't something many people ran (I assume).

(In reply to Randell Jesup [:jesup] from comment #16) > It may be WORKSFORME now, but that doesn't mean it isn't still there. > > I should note that there was no indication that this happened on *all* GIFs, > but that it happened on one particular GIF - which is not an unusual form > for such errors in image decoders; they're often data-dependent. We're well aware of that, that's why I ran the whole GIF collection we have in our fuzzing repository. > Also note that http://www.mozilla.org/projects/minefield/ has certainly > changed in the last 5 years, so going to that URL isn't a good test (if you > can get the state of that URL on the day/month or even year it was reported, > that'd be a better test. For the sake of completeness, I also ran the URL as it was at the time of the bug report through ASan, no errors (used Wayback Machine). I think it's likely that this is a false positive or bug in Purity.

Christian: excellent! It wasn't clear from the closing of this bug that we (you) were that thorough. Thanks.