Open Bug 847140 Opened 12 years ago Updated 2 years ago

ABORT: Height can't be negative!: 'aHeight >= 0', file /home/jwalden/moz/mfbt/image/src/Decoder.cpp, line 201 loading a BMP with height INT32_MIN

Categories

(Core :: Graphics: ImageLib, defect)

defect

Tracking

()

People

(Reporter: Waldo, Unassigned)

Details

The height recorded in a .bmp is a signed 32-bit integer: positive values produce an image with data laid out bottom to top, negative values produce an image with data laid out top to bottom.  The overall height of the image is abs(height in the image), which is fine except with height INT32_MIN.  Passing that through abs() has undefined behavior, but on twos-complement systems the value generally remains unchanged.  That "positive" value then propagates elsewhere, until eventually this abort/assert is hit.

The attached file is a 0xINT32_MIN bitmap, munged from a 0x0 bitmap with a hex editor.  I think this is a valid bitmap as the pixel data for 0*0 should also work for 0*|INT32_MIN|, although I could be wrong.  Technically, then, it should display as a valid image, but I could be wrong.  In any case rendering it shouldn't abort.

If you follow the code past the abort, I believe this calls RasterImage::SetSize(0, INT32_MIN), which will return an error because an input was negative, which will leave the width/height as they'd previously been -- 0x0.  Then further along we hit a RasterImage::EnsureFrame call with INT32_MIN height, that calls imgFrame::Init with negative height, and that errors out in AllowedImageSize when width/height are tested for <= 0.  So by virtue of backstopping I don't think there's an issue here except in debug builds.  Still, it's an issue worth fixing.  (And in the process we shouldn't be calling abs() on a value which could be the smallest possible signed integer of a type.)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.