From 4582797766b976cfd166ed598ad1d121b149a531 Mon Sep 17 00:00:00 2001 From: Jojakim Stahl Date: Thu, 22 Oct 2026 15:32:39 +0200 Subject: [PATCH 1/4] Fix heap overflow in the BMP RLE8 decoder (CVE-2020-21427) LoadPixelDataRLE8() let the position in the scanline grow beyond the image width: through the RLE_DELTA command, and by adding the full length of an absolute run even if only a part of it fits. After that, "width - bits" is negative, and the absolute mode passed it as the size to read_proc, which is converted to a huge unsigned value. The rest of the file is then written behind the scanline. - Keep the horizontal position inside [0, width] and the scanline inside [0, height] when applying RLE_DELTA, and when advancing after an absolute run. - Read into the scanline only the part of an absolute run which fits and skip the rest of the run. The same clamping is applied to encoded runs, which were already safe. - Do not call read_proc with a zero size. Add a TestAPI test that decodes valid RLE8 data, runs which are longer than the scanline, deltas outside of the image, and checks that truncated data is rejected. The test images are built in the test code. The test is built only if the BMP plugin is built in (see TEST_WITH_BMP). --- Source/Plugins/PluginBMP.cpp | 24 +++-- TestAPI/MainTestSuite.cpp | 7 ++ TestAPI/TestSuite.h | 8 ++ TestAPI/testBMP.cpp | 188 +++++++++++++++++++++++++++++++++++ 4 files changed, 220 insertions(+), 7 deletions(-) create mode 100644 TestAPI/testBMP.cpp diff --git a/Source/Plugins/PluginBMP.cpp b/Source/Plugins/PluginBMP.cpp index 12bc92c..827a688 100644 --- a/Source/Plugins/PluginBMP.cpp +++ b/Source/Plugins/PluginBMP.cpp @@ -387,8 +387,10 @@ LoadPixelDataRLE8(FreeImageIO *io, fi_handle handle, int width, int height, FIBI // apply them - bits += delta_x; - scanline += delta_y; + // keep the position inside of the image + + bits = std::min(bits + (int)delta_x, width); + scanline = std::min(scanline + (int)delta_y, abs(height)); break; } @@ -399,12 +401,20 @@ LoadPixelDataRLE8(FreeImageIO *io, fi_handle handle, int width, int height, FIBI return TRUE; } - int count = std::min((int)status_byte, width - bits); + // the run must not exceed the scanline, the rest of it is skipped + int count = std::max(std::min((int)status_byte, width - bits), 0); uint8_t *sline = FreeImage_GetScanLine(dib, scanline); - if (io->read_proc((void *)(sline + bits), sizeof(uint8_t) * count, 1, handle) != 1) { - return FALSE; + if (count > 0) { + if (io->read_proc((void *)(sline + bits), sizeof(uint8_t) * count, 1, handle) != 1) { + return FALSE; + } + } + for (int i = count; i < (int)status_byte; i++) { + if (io->read_proc(&second_byte, sizeof(uint8_t), 1, handle) != 1) { + return FALSE; + } } // align run length to even number of bytes @@ -415,7 +425,7 @@ LoadPixelDataRLE8(FreeImageIO *io, fi_handle handle, int width, int height, FIBI } } - bits += status_byte; + bits = std::min(bits + (int)status_byte, width); break; } @@ -429,7 +439,7 @@ LoadPixelDataRLE8(FreeImageIO *io, fi_handle handle, int width, int height, FIBI return TRUE; } - int count = std::min((int)status_byte, width - bits); + int count = std::max(std::min((int)status_byte, width - bits), 0); uint8_t *sline = FreeImage_GetScanLine(dib, scanline); diff --git a/TestAPI/MainTestSuite.cpp b/TestAPI/MainTestSuite.cpp index d9c81d8..25a015a 100644 --- a/TestAPI/MainTestSuite.cpp +++ b/TestAPI/MainTestSuite.cpp @@ -158,6 +158,13 @@ int main(int argc, char *argv[]) { testHeaderOnly(); #endif +#if TEST_WITH_BMP + // test BMP RLE8 decoding + if (!testBMPRLE8()) { + return 1; + } +#endif + // other tests testConvertToFloat(); testConvertToColor(); diff --git a/TestAPI/TestSuite.h b/TestAPI/TestSuite.h index 1659496..8016346 100644 --- a/TestAPI/TestSuite.h +++ b/TestAPI/TestSuite.h @@ -36,6 +36,13 @@ #endif #endif // WIN32 +// The BMP test needs the BMP plugin. FREEIMAGE_WITH_PLUGIN_BMP is set by the build +// system if the plugin can be disabled, otherwise the plugin is always built in. +#if !defined(FREEIMAGE_WITH_PLUGIN_BMP) || FREEIMAGE_WITH_PLUGIN_BMP +#define TEST_WITH_BMP 1 +#else +#define TEST_WITH_BMP 0 +#endif // Some useful tools // ========================================================== @@ -64,6 +71,7 @@ void testExifRaw(); // ========================================================== void testMemIO(const char *lpszPathName); +bool testBMPRLE8(); // Multipage test suite // ========================================================== diff --git a/TestAPI/testBMP.cpp b/TestAPI/testBMP.cpp new file mode 100644 index 0000000..cf651fd --- /dev/null +++ b/TestAPI/testBMP.cpp @@ -0,0 +1,188 @@ +// ========================================================== +// FreeImage 3 Test Script +// +// BMP: RLE8 decoding +// +// This file is part of FreeImage 3 +// +// COVERED CODE IS PROVIDED UNDER THIS LICENSE ON AN "AS IS" BASIS, WITHOUT WARRANTY +// OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING, WITHOUT LIMITATION, WARRANTIES +// THAT THE COVERED CODE IS FREE OF DEFECTS, MERCHANTABLE, FIT FOR A PARTICULAR PURPOSE +// OR NON-INFRINGING. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE OF THE COVERED +// CODE IS WITH YOU. SHOULD ANY COVERED CODE PROVE DEFECTIVE IN ANY RESPECT, YOU (NOT +// THE INITIAL DEVELOPER OR ANY OTHER CONTRIBUTOR) ASSUME THE COST OF ANY NECESSARY +// SERVICING, REPAIR OR CORRECTION. THIS DISCLAIMER OF WARRANTY CONSTITUTES AN ESSENTIAL +// PART OF THIS LICENSE. NO USE OF ANY COVERED CODE IS AUTHORIZED HEREUNDER EXCEPT UNDER +// THIS DISCLAIMER. +// +// Use at your own risk! +// ========================================================== + +#include "TestSuite.h" + +#if TEST_WITH_BMP + +#include +#include + +// ---------------------------------------------------------- + +static void push16(std::vector& v, unsigned value) { + v.push_back((uint8_t)(value & 0xFF)); + v.push_back((uint8_t)(value >> 8)); +} + +static void push32(std::vector& v, unsigned value) { + push16(v, value & 0xFFFF); + push16(v, value >> 16); +} + +/** +Build a bottom-up 8-bit BMP file with the RLE8 compressed pixel data. +The palette has 256 entries, entry i is (i, i, i). +*/ +static std::vector makeRLE8Bmp(int width, int height, const std::vector& rle) { + const unsigned headers = 14 + 40 + 256 * 4; + std::vector file; + file.push_back('B'); + file.push_back('M'); + push32(file, (unsigned)(headers + rle.size())); + push32(file, 0); + push32(file, headers); + push32(file, 40); // BITMAPINFOHEADER + push32(file, (unsigned)width); + push32(file, (unsigned)height); + push16(file, 1); // planes + push16(file, 8); // bit count + push32(file, 1); // BI_RLE8 + push32(file, (unsigned)rle.size()); + push32(file, 2835); + push32(file, 2835); + push32(file, 256); + push32(file, 0); + for (unsigned i = 0; i < 256; i++) { + file.push_back((uint8_t)i); + file.push_back((uint8_t)i); + file.push_back((uint8_t)i); + file.push_back(0); + } + file.insert(file.end(), rle.begin(), rle.end()); + return file; +} + +static FIBITMAP* loadBmpFromBuffer(std::vector& file) { + FIMEMORY *hmem = FreeImage_OpenMemory(file.data(), (uint32_t)file.size()); + FIBITMAP *dib = FreeImage_LoadFromMemory(FIF_BMP, hmem, 0); + FreeImage_CloseMemory(hmem); + return dib; +} + +static bool checkRow(FIBITMAP *dib, int scanline, const uint8_t *expected, int width) { + for (int x = 0; x < width; x++) { + uint8_t index = 0; + if (!FreeImage_GetPixelIndex(dib, (unsigned)x, (unsigned)scanline, &index) || index != expected[x]) { + return false; + } + } + return true; +} + +static bool expect(bool condition, const char *what) { + if (!condition) { + printf("testBMPRLE8: FAILED: %s\n", what); + } + return condition; +} + +// ---------------------------------------------------------- + +bool testBMPRLE8() { + printf("testBMPRLE8 ...\n"); + bool ok = true; + + // A valid image 6x2: scanline 0 is an encoded run, scanline 1 is an absolute run followed by an encoded run + { + const uint8_t rle[] = { + 6, 7, // 6 x index 7 + 0, 0, // end of line + 0, 3, 1, 2, 3, 0, // absolute run of 3 pixels (padded to an even number of bytes) + 3, 9, // 3 x index 9 + 0, 0, // end of line + 0, 1 // end of bitmap + }; + const uint8_t row0[6] = { 7, 7, 7, 7, 7, 7 }; + const uint8_t row1[6] = { 1, 2, 3, 9, 9, 9 }; + std::vector file = makeRLE8Bmp(6, 2, std::vector(rle, rle + sizeof(rle))); + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib != nullptr, "valid RLE8 is loaded"); + if (dib) { + ok &= expect(FreeImage_GetWidth(dib) == 6 && FreeImage_GetHeight(dib) == 2 && FreeImage_GetBPP(dib) == 8, "valid RLE8 dimensions"); + ok &= expect(checkRow(dib, 0, row0, 6), "valid RLE8 row 0"); + ok &= expect(checkRow(dib, 1, row1, 6), "valid RLE8 row 1"); + FreeImage_Unload(dib); + } + } + + // Runs which are longer than the scanline: the part which does not fit is skipped, the following data is decoded normally + { + const uint8_t rle[] = { + 4, 5, // 4 x index 5, the scanline is full + 0, 6, 11, 12, 13, 14, 15, 16, // absolute run of 6 pixels, nothing fits + 9, 6, // encoded run, nothing fits + 0, 0, // end of line + 0, 6, 21, 22, 23, 24, 25, 26, // absolute run of 6 pixels, only 4 fit + 0, 0, // end of line + 0, 1 + }; + const uint8_t row0[4] = { 5, 5, 5, 5 }; + const uint8_t row1[4] = { 21, 22, 23, 24 }; + std::vector file = makeRLE8Bmp(4, 2, std::vector(rle, rle + sizeof(rle))); + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib != nullptr, "over-long runs are loaded"); + if (dib) { + ok &= expect(checkRow(dib, 0, row0, 4), "over-long runs row 0"); + ok &= expect(checkRow(dib, 1, row1, 4), "over-long runs row 1"); + FreeImage_Unload(dib); + } + } + + // Delta commands leaving the image: the position is kept inside of the image + { + const uint8_t rle[] = { + 2, 3, // 2 x index 3 + 0, 2, 200, 1, // delta: far to the right, one line up + 0, 6, 1, 2, 3, 4, 5, 6, // absolute run, outside of the line + 0, 2, 0, 200, // delta: far above the image + 3, 4, // outside of the image, ends the decoding + 0, 1 + }; + std::vector file = makeRLE8Bmp(4, 2, std::vector(rle, rle + sizeof(rle))); + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib != nullptr, "delta outside of the image is handled"); + if (dib) { + const uint8_t row0[2] = { 3, 3 }; + ok &= expect(checkRow(dib, 0, row0, 2), "delta row 0"); + FreeImage_Unload(dib); + } + } + + // Truncated data: must be rejected + { + const uint8_t rle[] = { + 4, 5, + 0, 0, + 0, 6, 1, 2, 3 // the absolute run is cut off + }; + std::vector file = makeRLE8Bmp(4, 2, std::vector(rle, rle + sizeof(rle))); + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib == nullptr, "truncated RLE8 is rejected"); + if (dib) { + FreeImage_Unload(dib); + } + } + + printf(ok ? "testBMPRLE8: OK\n" : "testBMPRLE8: FAILED\n"); + return ok; +} + +#endif // TEST_WITH_BMP From 733dd8d43f40efe3d01eff68120605ac722cd1e1 Mon Sep 17 00:00:00 2001 From: Jojakim Stahl Date: Sat, 24 Oct 2026 21:31:17 +0200 Subject: [PATCH 2/4] BMP: Reject RLE data in images with a mismatching bit depth Idea from FreeImage SVN r1832 and r1836, reimplemented (the guard is a clear error instead of folding the check into the decoder call; applied to the Windows and the OS/2 2.x header paths). Trunk log r1832: "improved BMP plugin when working with malicious images" Trunk log r1836: "improved BMP plugin when working with malicious images" LoadPixelDataRLE8 writes up to width bytes per scanline and LoadPixelDataRLE4 writes width/2 bytes per scanline, but the image was allocated from the bit depth of the header. A BI_RLE8 file with a bit depth of 1 or 4, or a BI_RLE4 file with a bit depth of 1, therefore writes beyond the scanline. Both decoders are now only called for 8-bit and 4-bit images. Adds testBMPRLEBitDepth. --- Source/Plugins/PluginBMP.cpp | 12 +++++ TestAPI/MainTestSuite.cpp | 3 ++ TestAPI/TestSuite.h | 1 + TestAPI/testBMP.cpp | 94 ++++++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+) diff --git a/Source/Plugins/PluginBMP.cpp b/Source/Plugins/PluginBMP.cpp index 827a688..25178f9 100644 --- a/Source/Plugins/PluginBMP.cpp +++ b/Source/Plugins/PluginBMP.cpp @@ -553,6 +553,9 @@ LoadWindowsBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bit break; case BI_RLE4 : + if (bit_count != 4) { + throw "RLE4 compression requires a 4-bit BMP"; + } if ( LoadPixelDataRLE4(io, handle, width, height, dib.get()) ) { return dib.release(); } else { @@ -561,6 +564,9 @@ LoadWindowsBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bit break; case BI_RLE8 : + if (bit_count != 8) { + throw "RLE8 compression requires an 8-bit BMP"; + } if ( LoadPixelDataRLE8(io, handle, width, height, dib.get()) ) { return dib.release(); } else { @@ -772,6 +778,9 @@ LoadOS22XBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bits_ return dib.release(); case BI_RLE4 : + if (bit_count != 4) { + throw "RLE4 compression requires a 4-bit BMP"; + } if ( LoadPixelDataRLE4(io, handle, width, height, dib.get()) ) { return dib.release(); } else { @@ -780,6 +789,9 @@ LoadOS22XBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bits_ break; case BI_RLE8 : + if (bit_count != 8) { + throw "RLE8 compression requires an 8-bit BMP"; + } if ( LoadPixelDataRLE8(io, handle, width, height, dib.get()) ) { return dib.release(); } else { diff --git a/TestAPI/MainTestSuite.cpp b/TestAPI/MainTestSuite.cpp index 25a015a..c239cf0 100644 --- a/TestAPI/MainTestSuite.cpp +++ b/TestAPI/MainTestSuite.cpp @@ -163,6 +163,9 @@ int main(int argc, char *argv[]) { if (!testBMPRLE8()) { return 1; } + if (!testBMPRLEBitDepth()) { + return 1; + } #endif // other tests diff --git a/TestAPI/TestSuite.h b/TestAPI/TestSuite.h index 8016346..06a0b64 100644 --- a/TestAPI/TestSuite.h +++ b/TestAPI/TestSuite.h @@ -72,6 +72,7 @@ void testExifRaw(); void testMemIO(const char *lpszPathName); bool testBMPRLE8(); +bool testBMPRLEBitDepth(); // Multipage test suite // ========================================================== diff --git a/TestAPI/testBMP.cpp b/TestAPI/testBMP.cpp index cf651fd..bc86889 100644 --- a/TestAPI/testBMP.cpp +++ b/TestAPI/testBMP.cpp @@ -185,4 +185,98 @@ bool testBMPRLE8() { return ok; } +/** +Build a BMP file with a BITMAPINFOHEADER (40 bytes) and a 256 entry palette. +The pixel data is given as is, bit_count and compression are arbitrary. +*/ +static std::vector makeBmp(int width, int height, unsigned bit_count, unsigned compression, const std::vector& data) { + const unsigned headers = 14 + 40 + 256 * 4; + std::vector file; + file.push_back('B'); + file.push_back('M'); + push32(file, (unsigned)(headers + data.size())); + push32(file, 0); + push32(file, headers); + push32(file, 40); // BITMAPINFOHEADER + push32(file, (unsigned)width); + push32(file, (unsigned)height); + push16(file, 1); // planes + push16(file, bit_count); + push32(file, compression); + push32(file, (unsigned)data.size()); + push32(file, 2835); + push32(file, 2835); + push32(file, 0); + push32(file, 0); + for (unsigned i = 0; i < 256; i++) { + file.push_back((uint8_t)i); + file.push_back((uint8_t)i); + file.push_back((uint8_t)i); + file.push_back(0); + } + file.insert(file.end(), data.begin(), data.end()); + return file; +} + +/** +Runs of a width of 64 pixels, 4 lines: much more than a scanline of a 1- or 4-bit image can hold +*/ +static std::vector makeLongRuns() { + std::vector rle; + for (int y = 0; y < 4; y++) { + rle.push_back(64); + rle.push_back(0x11); + rle.push_back(0); + rle.push_back(0); + } + rle.push_back(0); + rle.push_back(1); + return rle; +} + +// ---------------------------------------------------------- + +bool testBMPRLEBitDepth() { + printf("testBMPRLEBitDepth ...\n"); + bool ok = true; + + // RLE8 and RLE4 data in a file whose bit depth is not 8 or 4: the decoders write more + // pixels per scanline than the allocated image can hold, so the file must be rejected + const struct { unsigned bit_count; unsigned compression; const char *what; } bad[] = { + { 1, 1, "RLE8 with 1-bit depth is rejected" }, + { 4, 1, "RLE8 with 4-bit depth is rejected" }, + { 1, 2, "RLE4 with 1-bit depth is rejected" }, + { 8, 2, "RLE4 with 8-bit depth is rejected" }, + }; + for (const auto& t : bad) { + std::vector file = makeBmp(64, 4, t.bit_count, t.compression, makeLongRuns()); + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib == nullptr, t.what); + if (dib) { + FreeImage_Unload(dib); + } + } + + // the matching combinations are still decoded + { + std::vector file = makeBmp(64, 4, 8, 1, makeLongRuns()); + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib != nullptr && FreeImage_GetBPP(dib) == 8, "RLE8 with 8-bit depth is loaded"); + if (dib) { + FreeImage_Unload(dib); + } + } + { + std::vector file = makeBmp(64, 4, 4, 2, makeLongRuns()); + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib != nullptr && FreeImage_GetBPP(dib) == 4, "RLE4 with 4-bit depth is loaded"); + if (dib) { + FreeImage_Unload(dib); + } + } + + printf(ok ? "testBMPRLEBitDepth: OK\n" : "testBMPRLEBitDepth: FAILED\n"); + return ok; +} + #endif // TEST_WITH_BMP From 2eff473a01872d8fa9f5967a52df32a191f5abfd Mon Sep 17 00:00:00 2001 From: Jojakim Stahl Date: Sat, 24 Oct 2026 21:33:27 +0200 Subject: [PATCH 3/4] BMP: Check the results of the header reads Idea from FreeImage SVN r1832 and r1836, reimplemented (FreeImageRe reports the error with a local message and no FI_MSG_ERROR_INVALID_FORMAT exists here; the OS/2 2.x header path is covered as well). Trunk log r1832: "improved BMP plugin when working with malicious images" Trunk log r1836: "improved BMP plugin when working with malicious images" The results of the reads of the file header, the BITMAPINFOHEADER and the OS/2 headers were ignored, so a file which ended inside of a header was processed with uninitialized header fields. The structures are now cleared and a short read ends the loading. Adds testBMPTruncatedHeaders. --- Source/Plugins/PluginBMP.cpp | 21 +++++++++++--- TestAPI/MainTestSuite.cpp | 3 ++ TestAPI/TestSuite.h | 1 + TestAPI/testBMP.cpp | 54 ++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/Source/Plugins/PluginBMP.cpp b/Source/Plugins/PluginBMP.cpp index 25178f9..f85f727 100644 --- a/Source/Plugins/PluginBMP.cpp +++ b/Source/Plugins/PluginBMP.cpp @@ -469,8 +469,11 @@ LoadWindowsBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bit // load the info header FIBITMAPINFOHEADER bih; + memset(&bih, 0, sizeof(bih)); - io->read_proc(&bih, sizeof(FIBITMAPINFOHEADER), 1, handle); + if (io->read_proc(&bih, sizeof(FIBITMAPINFOHEADER), 1, handle) != 1) { + throw "BMP info header is truncated"; + } #ifdef FREEIMAGE_BIGENDIAN SwapInfoHeader(&bih); #endif @@ -691,8 +694,11 @@ LoadOS22XBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bits_ // load the info header FIBITMAPINFOHEADER bih; + memset(&bih, 0, sizeof(bih)); - io->read_proc(&bih, sizeof(FIBITMAPINFOHEADER), 1, handle); + if (io->read_proc(&bih, sizeof(FIBITMAPINFOHEADER), 1, handle) != 1) { + throw "BMP info header is truncated"; + } #ifdef FREEIMAGE_BIGENDIAN SwapInfoHeader(&bih); #endif @@ -895,7 +901,10 @@ LoadOS21XBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bits_ BITMAPINFOOS2_1X_HEADER bios2_1x; - io->read_proc(&bios2_1x, sizeof(BITMAPINFOOS2_1X_HEADER), 1, handle); + memset(&bios2_1x, 0, sizeof(bios2_1x)); + if (io->read_proc(&bios2_1x, sizeof(BITMAPINFOOS2_1X_HEADER), 1, handle) != 1) { + throw "BMP info header is truncated"; + } #ifdef FREEIMAGE_BIGENDIAN SwapOS21XHeader(&bios2_1x); #endif @@ -1106,7 +1115,11 @@ Load(FreeImageIO *io, fi_handle handle, int page, int flags, void *data) { // read the fileheader - io->read_proc(&bitmapfileheader, sizeof(BITMAPFILEHEADER), 1, handle); + memset(&bitmapfileheader, 0, sizeof(bitmapfileheader)); + if (io->read_proc(&bitmapfileheader, sizeof(BITMAPFILEHEADER), 1, handle) != 1) { + FreeImage_OutputMessageProc(s_format_id, "BMP file header is truncated"); + return nullptr; + } #ifdef FREEIMAGE_BIGENDIAN SwapFileHeader(&bitmapfileheader); #endif diff --git a/TestAPI/MainTestSuite.cpp b/TestAPI/MainTestSuite.cpp index c239cf0..99d3376 100644 --- a/TestAPI/MainTestSuite.cpp +++ b/TestAPI/MainTestSuite.cpp @@ -166,6 +166,9 @@ int main(int argc, char *argv[]) { if (!testBMPRLEBitDepth()) { return 1; } + if (!testBMPTruncatedHeaders()) { + return 1; + } #endif // other tests diff --git a/TestAPI/TestSuite.h b/TestAPI/TestSuite.h index 06a0b64..d905fa4 100644 --- a/TestAPI/TestSuite.h +++ b/TestAPI/TestSuite.h @@ -73,6 +73,7 @@ void testExifRaw(); void testMemIO(const char *lpszPathName); bool testBMPRLE8(); bool testBMPRLEBitDepth(); +bool testBMPTruncatedHeaders(); // Multipage test suite // ========================================================== diff --git a/TestAPI/testBMP.cpp b/TestAPI/testBMP.cpp index bc86889..a21d9df 100644 --- a/TestAPI/testBMP.cpp +++ b/TestAPI/testBMP.cpp @@ -279,4 +279,58 @@ bool testBMPRLEBitDepth() { return ok; } +bool testBMPTruncatedHeaders() { + printf("testBMPTruncatedHeaders ...\n"); + bool ok = true; + + std::vector pixels(4 * 4, 3); + const std::vector valid = makeBmp(4, 4, 8, 0, pixels); + { + std::vector file = valid; + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib != nullptr, "unmodified BMP is loaded"); + if (dib) { + FreeImage_Unload(dib); + } + } + + // the file header is 14 bytes, the info header 40 bytes: every file which ends in one + // of the headers must be rejected + const size_t cuts[] = { 0, 1, 2, 13, 14, 15, 18, 30, 53 }; + for (size_t cut : cuts) { + std::vector file(valid.begin(), valid.begin() + cut); + FIBITMAP *dib = nullptr; + if (!file.empty()) { + dib = loadBmpFromBuffer(file); + } + char what[64]; + snprintf(what, sizeof(what), "BMP cut after %u bytes is rejected", (unsigned)cut); + ok &= expect(dib == nullptr, what); + if (dib) { + FreeImage_Unload(dib); + } + } + + // OS/2 1.x header (12 bytes) cut inside of the header + { + std::vector file; + file.push_back('B'); + file.push_back('M'); + push32(file, 14 + 12); + push32(file, 0); + push32(file, 14 + 12); + push32(file, 12); // BITMAPCOREHEADER + push16(file, 4); // width + file.push_back(4); // only the first byte of the height + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib == nullptr, "OS/2 1.x BMP cut inside of the header is rejected"); + if (dib) { + FreeImage_Unload(dib); + } + } + + printf(ok ? "testBMPTruncatedHeaders: OK\n" : "testBMPTruncatedHeaders: FAILED\n"); + return ok; +} + #endif // TEST_WITH_BMP From 8503249ae328e5eb61acd889ae86b0c136a31a53 Mon Sep 17 00:00:00 2001 From: Jojakim Stahl Date: Sat, 24 Oct 2026 21:35:47 +0200 Subject: [PATCH 4/4] BMP: Validate the info header, accepting all header versions Idea from FreeImage SVN r1832 and r1836 (CheckBitmapInfoHeader), reimplemented: trunk rejects every biSize other than 40, which makes it refuse all BITMAPV4HEADER (108) and BITMAPV5HEADER (124) files and the versions 52, 56 and the OS/2 2.x header (64) which Load dispatches here. The negative width stays checked in LoadWindowsBMP. The planes field is not checked, as many writers store 0. Trunk log r1832: "improved BMP plugin when working with malicious images" Trunk log r1836: "improved BMP plugin when working with malicious images" CheckBitmapInfoHeader accepts the six header sizes, the bit depths 1, 4, 8, 16, 24 and 32, and rejects RLE compressed top-down images. Adds testBMPHeaderVersions, which loads 40, 108 and 124 byte header files with 24 and 32 bits. --- Source/Plugins/PluginBMP.cpp | 48 ++++++++++++++ TestAPI/MainTestSuite.cpp | 3 + TestAPI/TestSuite.h | 1 + TestAPI/testBMP.cpp | 121 +++++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+) diff --git a/Source/Plugins/PluginBMP.cpp b/Source/Plugins/PluginBMP.cpp index f85f727..7cdd467 100644 --- a/Source/Plugins/PluginBMP.cpp +++ b/Source/Plugins/PluginBMP.cpp @@ -461,6 +461,48 @@ LoadPixelDataRLE8(FreeImageIO *io, fi_handle handle, int width, int height, FIBI // -------------------------------------------------------------------------- +/** +Check the fields of a BITMAPINFOHEADER which are needed to decode the image. +The header size has to be one of the versions which are accepted by Load: +BITMAPINFOHEADER (40), BITMAPV2INFOHEADER (52), BITMAPV3INFOHEADER (56), the OS/2 2.x header (64), +BITMAPV4HEADER (108) and BITMAPV5HEADER (124). The fields of the version 4 and 5 headers beyond +the first 40 bytes are not used here. +The negative width is checked by the callers. +@return Returns NULL if the header is valid, otherwise the error message +*/ +static const char * +CheckBitmapInfoHeader(const FIBITMAPINFOHEADER *bih) { + switch (bih->biSize) { + case 40: + case 52: + case 56: + case 64: + case 108: + case 124: + break; + default: + return "Unsupported BMP header size"; + } + switch (bih->biBitCount) { + case 1: + case 4: + case 8: + case 16: + case 24: + case 32: + break; + default: + return "Unsupported BMP bit depth"; + } + if (bih->biHeight < 0) { + // a top-down DIB cannot be compressed with RLE + if ((bih->biCompression == BI_RLE4) || (bih->biCompression == BI_RLE8)) { + return "A top-down BMP cannot be RLE compressed"; + } + } + return nullptr; +} + static FIBITMAP * LoadWindowsBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bits_offset, int type) { try { @@ -477,6 +519,9 @@ LoadWindowsBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bit #ifdef FREEIMAGE_BIGENDIAN SwapInfoHeader(&bih); #endif + if (const char *error = CheckBitmapInfoHeader(&bih)) { + throw error; + } // keep some general information about the bitmap @@ -702,6 +747,9 @@ LoadOS22XBMP(FreeImageIO *io, fi_handle handle, int flags, unsigned bitmap_bits_ #ifdef FREEIMAGE_BIGENDIAN SwapInfoHeader(&bih); #endif + if (const char *error = CheckBitmapInfoHeader(&bih)) { + throw error; + } // keep some general information about the bitmap diff --git a/TestAPI/MainTestSuite.cpp b/TestAPI/MainTestSuite.cpp index 99d3376..22fb4e3 100644 --- a/TestAPI/MainTestSuite.cpp +++ b/TestAPI/MainTestSuite.cpp @@ -169,6 +169,9 @@ int main(int argc, char *argv[]) { if (!testBMPTruncatedHeaders()) { return 1; } + if (!testBMPHeaderVersions()) { + return 1; + } #endif // other tests diff --git a/TestAPI/TestSuite.h b/TestAPI/TestSuite.h index d905fa4..2dbdd73 100644 --- a/TestAPI/TestSuite.h +++ b/TestAPI/TestSuite.h @@ -74,6 +74,7 @@ void testMemIO(const char *lpszPathName); bool testBMPRLE8(); bool testBMPRLEBitDepth(); bool testBMPTruncatedHeaders(); +bool testBMPHeaderVersions(); // Multipage test suite // ========================================================== diff --git a/TestAPI/testBMP.cpp b/TestAPI/testBMP.cpp index a21d9df..e815da6 100644 --- a/TestAPI/testBMP.cpp +++ b/TestAPI/testBMP.cpp @@ -333,4 +333,125 @@ bool testBMPTruncatedHeaders() { return ok; } +/** +Build a 4x2 bottom-up BMP with the info header of the given version (40, 108 or 124 bytes). +24 bit: BI_RGB, 32 bit: BI_BITFIELDS with the usual BGRA masks. +Pixel (x, y) in the top row has the color (10 + x, 20 + x, 30 + x), the bottom row (110 + x, 120 + x, 130 + x). +*/ +static std::vector makeVersionedBmp(unsigned header_size, unsigned bit_count) { + const int width = 4, height = 2; + const unsigned bytes = bit_count / 8; + const unsigned pitch = (width * bytes + 3) & ~3u; + std::vector pixels(pitch * height, 0); + for (int y = 0; y < height; y++) { + // the file is bottom-up: the first row of the file is the bottom row + const unsigned base = (y == 0) ? 110 : 10; + for (int x = 0; x < width; x++) { + uint8_t *p = &pixels[y * pitch + x * bytes]; + p[0] = (uint8_t)(base + 20 + x); // blue + p[1] = (uint8_t)(base + 10 + x); // green + p[2] = (uint8_t)(base + x); // red + if (bytes == 4) { + p[3] = 255; + } + } + } + const unsigned offset = 14 + header_size; + std::vector file; + file.push_back('B'); + file.push_back('M'); + push32(file, (unsigned)(offset + pixels.size())); + push32(file, 0); + push32(file, offset); + push32(file, header_size); + push32(file, (unsigned)width); + push32(file, (unsigned)height); + push16(file, 1); + push16(file, bit_count); + push32(file, bit_count == 32 ? 3u : 0u); // BI_BITFIELDS or BI_RGB + push32(file, (unsigned)pixels.size()); + push32(file, 2835); + push32(file, 2835); + push32(file, 0); + push32(file, 0); + if (header_size >= 108) { + push32(file, 0x00FF0000); // red mask + push32(file, 0x0000FF00); // green mask + push32(file, 0x000000FF); // blue mask + push32(file, bit_count == 32 ? 0xFF000000u : 0u); // alpha mask + push32(file, 0x73524742); // 'sRGB' + while (file.size() < 14 + header_size) { + file.push_back(0); // endpoints, gamma, and the version 5 fields + } + } + file.insert(file.end(), pixels.begin(), pixels.end()); + return file; +} + +bool testBMPHeaderVersions() { + printf("testBMPHeaderVersions ...\n"); + bool ok = true; + + const unsigned sizes[] = { 40, 108, 124 }; + const unsigned depths[] = { 24, 32 }; + for (unsigned header_size : sizes) { + for (unsigned bit_count : depths) { + char what[96]; + snprintf(what, sizeof(what), "BMP with a %u byte header, %u bit, is loaded", header_size, bit_count); + std::vector file = makeVersionedBmp(header_size, bit_count); + FIBITMAP *dib = loadBmpFromBuffer(file); + bool good = (dib != nullptr) && FreeImage_GetWidth(dib) == 4 && FreeImage_GetHeight(dib) == 2 && FreeImage_GetBPP(dib) == bit_count; + if (good) { + // scanline 0 of the dib is the bottom row of the image + for (unsigned y = 0; y < 2; y++) { + const unsigned base = (y == 0) ? 110 : 10; + for (unsigned x = 0; x < 4; x++) { + FIRGBA8 color; + good &= FreeImage_GetPixelColor(dib, x, y, &color) != 0; + good &= color.red == base + x && color.green == base + 10 + x && color.blue == base + 20 + x; + } + } + } + ok &= expect(good, what); + if (dib) { + FreeImage_Unload(dib); + } + } + } + + // header fields which cannot be decoded are rejected + { + std::vector file = makeVersionedBmp(40, 24); + file[28] = 2; // biBitCount = 2 + file[29] = 0; + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib == nullptr, "BMP with a bit depth of 2 is rejected"); + if (dib) { + FreeImage_Unload(dib); + } + } + { + std::vector file = makeVersionedBmp(40, 24); + file[14] = 44; // biSize = 44, no such header + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib == nullptr, "BMP with an unknown header size is rejected"); + if (dib) { + FreeImage_Unload(dib); + } + } + { + // top-down and RLE8 compressed + std::vector rle = { 4, 1, 0, 0, 4, 2, 0, 0, 0, 1 }; + std::vector file = makeBmp(4, -2, 8, 1, rle); + FIBITMAP *dib = loadBmpFromBuffer(file); + ok &= expect(dib == nullptr, "top-down RLE8 BMP is rejected"); + if (dib) { + FreeImage_Unload(dib); + } + } + + printf(ok ? "testBMPHeaderVersions: OK\n" : "testBMPHeaderVersions: FAILED\n"); + return ok; +} + #endif // TEST_WITH_BMP