243511 - crash on BMP image

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Mike Shaver (:shaver emeritus)]

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.

Comment 1

[Mike Shaver (:shaver emeritus)]

Attachment: BMP image (CRASHES 2004-05-13 07:09 PDT TRUNK)

I also crash with Tbird 0.6, probably unsurprising.

Comment 2

[[e-mail]]

Attachment: Validate mBIH.Colors

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 243493 has been marked as a duplicate of this bug. ***

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 243529 has been marked as a duplicate of this bug. ***

Comment 5

[Bernard Alleysson]

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.

Comment 6

[Bernard Alleysson]

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.

Comment 7

[[e-mail]]

Comment on attachment 148413 [details] [diff] [review]
Validate mBIH.Colors

Low-risk crash fix

Comment 8

[chris hofmann]

Comment on attachment 148413 [details] [diff] [review]
Validate mBIH.Colors

a=chofmann for 1.7

Comment 9

[[e-mail]]

Someone should change the title of attachment 148412 [details] ;-)

Comment 10

[Tracy Walker [:tracy]]

verified