[jafin]

Prerequisites

• I have written a descriptive pull-request title
• I have verified that there are no overlapping pull-requests open
• I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
• I have provided test coverage for my change (where applicable)

Description

This PR fixes two problems in how BmpDecoderCore handles malformed headers.

Invalid bit depth for an uncompressed bitmap

A BI_RGB bitmap with a bit depth other than 1, 2, 4, 8, 16, 24 or 32 had no consistent handling:

• Image.Identify accepted it. With a depth of 0 it reported 0 bits per pixel.
• Image.Load failed with a different exception for each depth: InvalidImageContentException for 0 or 33, ImageFormatException (wrapping an IndexOutOfRangeException from the palette reader) for 3, 5 or 7, and NotSupportedException for 12.

ReadImageHeaders now rejects any other depth for BI_RGB with InvalidImageContentException. Identify and Load go through that method, so both now fail the same way. The RGB when bitsPerPixel is <= 0 or > 32 case in Decode can no longer be reached, so I removed it.

Pixel data smaller than the header declares

The decoder allocated the full pixel buffer before it read any pixel data. A 126-byte file that declared 4000x4000 at 24 bpp requested a 64 MB buffer before the first row read failed. At 10000x10000 the request was about 400 MB. The row readers only fail when a read returns no bytes. Because of this, a file that declared one 100000-pixel row with 72 bytes of data decoded without an error.

Before it allocates the image, Decode now computes the pixel data size that the header declares for uncompressed (BI_RGB) and bit fields bitmaps. If the remaining stream is shorter, it throws InvalidImageContentException. The calculation uses long arithmetic, so very large declared dimensions do not overflow. The padding of the last row can still be missing, as it could before. RLE data has no fixed size, so this check does not apply to it.

Testing

New tests in BmpDecoderTests:

• BmpDecoder_InvalidUncompressedBitsPerPixel_ThrowsInvalidImageContentException covers depths 0, 3, 7, 12 and 33. It checks both Identify and Load.
• BmpDecoder_PixelDataSmallerThanDeclared_ThrowsBeforeAllocatingPixelBuffer covers 4000x4000 and 100000x1 with 72 bytes of pixel data. It uses TestMemoryAllocator to check that no buffer the size of the declared pixel buffer is requested.
• BmpDecoder_MissingLastRowPadding_Decodes checks that a file without the padding of its last row still decodes.

The test bitmaps are built in memory. No test images were added.

The first two tests fail on main and pass with this change. The third test passes on both.

I ran these tests in Release on net10.0:

• All tests in the Bmp, Ico, Cur and Ani test namespaces: 387 passed.
• GeneralFormatTests, InvalidImageDimensionsTests and the ImageTests* classes: 295 passed.

Fixes #3235

[JimBobSquarePants]

Hi @jafin thank you for these contributions. Could you please raise relevant issues, so we have something to track for release notes.

[jafin]

@JimBobSquarePants no worries, Issues are there now