#654 Check on number colors

[wqwang]

I'm running identify on the image attached.

GraphicsMagick succeeds in parsing it, while ImageMagick complains

InsufficientImageDataInFile ... error/bmp.c/ReadBMPImage/673.

The corresponding check in ImageMagick is

if ((MagickSizeType) bmp_info.number_colors > blob_size)
    ThrowReaderException(CorruptImageError,"InsufficientImageDataInFile");

It seems the bmp parser in GraphicsMagick doesn't check number colors. I'm wondering whether if there is a missing check on number_colors in GraphicsMagick's BMP parser.

[Bob Friesenhahn]

On Mon, 29 Nov 2021, Weiqi Wang wrote:

GraphicsMagick succeeds in parsing it, while ImageMagick complains
InsufficientImageDataInFile ... error/bmp.c/ReadBMPImage/673.

The corresponding check in ImageMagick is
if ((MagickSizeType) bmp_info.number_colors > blob_size) ThrowReaderException(CorruptImageError,"InsufficientImageDataInFile");

It seems the bmp parser in GraphicsMagick doesn't check number colors. I'm wondering whether if there is a missing check on number_colors in GraphicsMagick's BMP parser.

You are describing a value judgement made by an ImageMagick developer
in order to try to solve a security issue. GraphicsMagick developers
chose to use other factors to validate the file. This does not mean
there is a problem.

FYI, when using 'identify' normally only the file header is looked at
without reading the image data. Often it is useful to report what the
file header says, even if it is incorrect.

With GraphicsMagick do

gm identify +ping foo.bmp

in order to cause it to read the entire file.

Bob

Bob Friesenhahn
bfriesen@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer, http://www.GraphicsMagick.org/
Public Key, http://www.simplesystems.org/users/bfriesen/public-key.txt

[Bob Friesenhahn]

The BMP number_colors parameter is not always relevant since using it while decoding depends on the BMP version and the number of bits per pixel.

There is a check for number_of colors at line 951 of bmp.c:

if (bmp_info.bits_per_pixel < 16)
  {
    if (bmp_info.number_colors > (1UL << bmp_info.bits_per_pixel))
      ThrowBMPReaderException(CorruptImageError,UnrecognizedNumberOfColors,image);
  }

It would be bad to reject otherwise valid files since then users would be blocked from reading their files.

I don't think there is a problem here to fix.