Closed Bug 243511 Opened 20 years ago Closed 20 years ago

crash on BMP image

Categories

(Core :: Graphics: ImageLib, defect)

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: shaver, Assigned: neil)

References

Details

(Keywords: crash, verified1.7)

Attachments

(2 files)

I was crashing with in the BMP decoder with this image, found in email (spam,
natch).  I have a non-debug stack trace, pointing at some bogus memory
allocation behaviour:

#4  0x00a91044 in std::terminate () from /usr/lib/libstdc++.so.5
#5  0x00a911b6 in __cxa_throw () from /usr/lib/libstdc++.so.5
#6  0x00a9140f in operator new () from /usr/lib/libstdc++.so.5
#7  0x00a914df in operator new[] () from /usr/lib/libstdc++.so.5
#8  0x0044a98f in nsBMPDecoder::ProcessData ()
   from
/home/shaver/mozilla/tbird-obj-i686-pc-linux-gnu/dist/bin/components/libimglib2.so
#9  0x0044a4bb in nsBMPDecoder::ReadSegCb ()
   from
/home/shaver/mozilla/tbird-obj-i686-pc-linux-gnu/dist/bin/components/libimglib2.so
#10 0x00ee2f84 in nsPipeInputStream::ReadSegments () from ./libxpcom.so
#11 0x0044a4eb in nsBMPDecoder::WriteFrom ()
   from
/home/shaver/mozilla/tbird-obj-i686-pc-linux-gnu/dist/bin/components/libimglib2.so
#12 0x0044354b in imgRequest::OnDataAvailable ()
   from
/home/shaver/mozilla/tbird-obj-i686-pc-linux-gnu/dist/bin/components/libimglib2.so

etc.
I also crash with Tbird 0.6, probably unsurprising.
Assignee: jdunn → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Attachment #148413 - Flags: review?(cbiesinger)
*** Bug 243493 has been marked as a duplicate of this bug. ***
Attachment #148413 - Flags: superreview?(tor)
Flags: blocking1.7+
*** Bug 243529 has been marked as a duplicate of this bug. ***
The patch does not address the problem, for instance what happens if
the corrupt BMP header is such that 
mBIH.colors = 0
mBIH.bpp = 31
Then with the patch you end up allocating 1 << 31 bytes ===> CRASH again

The problem here is that operator new() throws an exception because the BMP
header is corrupt thus causig a very large allocation (note that this is not
crashing Windows). Maybe a good solution here is to use malloc() instead of
new() for this particular allocation point, assuming that malloc doesn't throw.
I don't know if you can use std::nothrow.

Sorry, I didn't look at the context, my example can't happen:
because of "if (mBIH.bpp <= 8) {" the maximum for mColors is 1 << 8 so the patch
is ok.
Attachment #148413 - Flags: superreview?(tor) → superreview+
Attachment #148413 - Flags: review?(cbiesinger) → review+
Comment on attachment 148413 [details] [diff] [review]
Validate mBIH.Colors

Low-risk crash fix
Attachment #148413 - Flags: approval1.7?
Comment on attachment 148413 [details] [diff] [review]
Validate mBIH.Colors

a=chofmann for 1.7
Attachment #148413 - Flags: approval1.7? → approval1.7+
Someone should change the title of attachment 148412 [details] ;-)
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Keywords: crash, fixed1.7
Resolution: --- → FIXED
Attachment #148412 - Attachment description: BMP image (CRASHES CURRENT TRUNK) → BMP image (CRASHES 2004-05-13 07:09 PDT TRUNK)
verified
Status: RESOLVED → VERIFIED
Keywords: fixed1.7verified1.7
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: