Fix integer overflow in image decoders

Author: nidu-ninja

Tests/LibGfx/TestImageDecoder.cpp

@@ -2338,3 +2338,60 @@ TEST_CASE(test_dicom)
         for (int x = 0; x < frame.image->width(); ++x)
             EXPECT_EQ(frame.image->get_pixel(x, y), reference_frame.image->get_pixel(x, y));
 }
+
+// Regression tests for integer-overflow fixes in image decoders.
+// Each malformed file exercises a specific arithmetic-sanitisation guard and
+// must be decoded without crashing and without allocating runaway memory.
+
+TEST_CASE(test_tiff_strip_count_overflow)
+{
+    // ImageLength=10, RowsPerStrip=7, but 4 StripOffsets supplied.
+    // segment_index 2 → tile_row 2 → rows_used = 7*2 = 14 > 10 → Error before any decode.
+    Array test_inputs = {
+        TEST_INPUT("tiff/malformed/tiff_strip_overflow.tiff"sv),
+    };
+    for (auto path : test_inputs) {
+        auto file = TRY_OR_FAIL(Core::MappedFile::map(path));
+        auto plugin_or_error = Gfx::TIFFImageDecoderPlugin::create(file->bytes());
+        if (plugin_or_error.is_error())
+            continue;
+        // frame(0) must return an error — not crash, not hang
+        auto frame_or_error = plugin_or_error.value()->frame(0);
+        EXPECT(frame_or_error.is_error());
+    }
+}
+
+TEST_CASE(test_tiff_ccitt_huge_dimensions)
+{
+    // ImageWidth=ImageLength=65535, CCITT-RLE compression.
+    // 65535*65535 = 4,294,836,225 > UINT32_MAX → Checked<u32> overflow → Error.
+    Array test_inputs = {
+        TEST_INPUT("tiff/malformed/tiff_ccitt_huge_dimensions.tiff"sv),
+    };
+    for (auto path : test_inputs) {
+        auto file = TRY_OR_FAIL(Core::MappedFile::map(path));
+        auto plugin_or_error = Gfx::TIFFImageDecoderPlugin::create(file->bytes());
+        if (plugin_or_error.is_error())
+            continue;
+        auto frame_or_error = plugin_or_error.value()->frame(0);
+        EXPECT(frame_or_error.is_error());
+    }
+}
+
+TEST_CASE(test_bmp_rle_buffer_size_overflow)
+{
+    // width=65536, height=65536, RLE8: 65536*65536 wraps u32 → Checked<u32> catches it.
+    // width=0x3FFFFFFF, height=2, RLE24: round_up(w,4)*4 overflows u32 → caught.
+    Array test_inputs = {
+        TEST_INPUT("bmp/malformed/bmp_rle8_overflow.bmp"sv),
+        TEST_INPUT("bmp/malformed/bmp_rle24_overflow.bmp"sv),
+    };
+    for (auto path : test_inputs) {
+        auto file = TRY_OR_FAIL(Core::MappedFile::map(path));
+        auto plugin_or_error = Gfx::BMPImageDecoderPlugin::create(file->bytes());
+        if (plugin_or_error.is_error())
+            continue;
+        auto frame_or_error = plugin_or_error.value()->frame(0);
+        EXPECT(frame_or_error.is_error());
+    }
+}

Userland/Libraries/LibGfx/ImageFormats/BMPLoader.cpp

@@ -7,6 +7,7 @@
 
 #include <AK/BuiltinWrappers.h>
 #include <AK/ByteString.h>
+#include <AK/Checked.h>
 #include <AK/Debug.h>
 #include <AK/Error.h>
 #include <AK/Function.h>
@@ -1023,16 +1024,15 @@ static ErrorOr<void> uncompress_bmp_rle_data(BMPLoadingContext& context, ByteBuf
     // ByteBuffer asserts that allocating the memory never fails.
     // FIXME: ByteBuffer should return either RefPtr<> or Optional<>.
     // Decoding the RLE data on-the-fly might actually be faster, and avoids this topic entirely.
-    u32 buffer_size;
-    if (compression == Compression::RLE24) {
-        buffer_size = total_rows * round_up_to_power_of_two(total_columns, 4) * 4;
-    } else {
-        buffer_size = total_rows * round_up_to_power_of_two(total_columns, 4);
-    }
-    if (buffer_size > 300 * MiB) {
+    Checked<u32> checked_buffer_size = total_rows;
+    checked_buffer_size *= round_up_to_power_of_two(total_columns, 4);
+    if (compression == Compression::RLE24)
+        checked_buffer_size *= 4;
+    if (checked_buffer_size.has_overflow() || checked_buffer_size.value() > 300 * MiB) {
         dbgln("Suspiciously large amount of RLE data");
         return Error::from_string_literal("Suspiciously large amount of RLE data");
     }
+    u32 buffer_size = checked_buffer_size.value();
     auto buffer_result = ByteBuffer::create_zeroed(buffer_size);
     if (buffer_result.is_error()) {
         dbgln("Not enough memory for buffer allocation");

Userland/Libraries/LibGfx/ImageFormats/CCITTDecoder.cpp

@@ -6,6 +6,7 @@
 
 #include <AK/Array.h>
 #include <AK/BitStream.h>
+#include <AK/Checked.h>
 #include <AK/MemoryStream.h>
 #include <LibGfx/Color.h>
 #include <LibGfx/ImageFormats/CCITTCommon.h>
@@ -343,7 +344,15 @@ ErrorOr<ByteBuffer> decode_ccitt_rle(ReadonlyBytes bytes, u32 image_width, u32 i
     auto bit_stream = make<BigEndianInputBitStream>(MaybeOwned<Stream>(*strip_stream));
 
     // Note: We put image_height extra-space to handle at most one alignment to byte boundary per line.
-    ByteBuffer decoded_bytes = TRY(ByteBuffer::create_zeroed(ceil_div(image_width * image_height, 8) + image_height));
+    Checked<u32> pixel_count = image_width;
+    pixel_count *= image_height;
+    if (pixel_count.has_overflow())
+        return Error::from_string_literal("CCITTDecoder: image dimensions too large");
+    Checked<u32> allocation_size = pixel_count.value() / 8 + (pixel_count.value() % 8 != 0 ? 1 : 0);
+    allocation_size += image_height;
+    if (allocation_size.has_overflow())
+        return Error::from_string_literal("CCITTDecoder: image dimensions too large");
+    ByteBuffer decoded_bytes = TRY(ByteBuffer::create_zeroed(allocation_size.value()));
     auto output_stream = make<FixedMemoryStream>(decoded_bytes.bytes());
     auto decoded_bits = make<BigEndianOutputBitStream>(MaybeOwned<Stream>(*output_stream));
 
@@ -362,7 +371,15 @@ ErrorOr<ByteBuffer> decode_ccitt_group3(ReadonlyBytes bytes, u32 image_width, u3
     auto bit_stream = make<BigEndianInputBitStream>(MaybeOwned<Stream>(*strip_stream));
 
     // Note: We put image_height extra-space to handle at most one alignment to byte boundary per line.
-    ByteBuffer decoded_bytes = TRY(ByteBuffer::create_zeroed(ceil_div(image_width * image_height, 8) + image_height));
+    Checked<u32> pixel_count = image_width;
+    pixel_count *= image_height;
+    if (pixel_count.has_overflow())
+        return Error::from_string_literal("CCITTDecoder: image dimensions too large");
+    Checked<u32> allocation_size = pixel_count.value() / 8 + (pixel_count.value() % 8 != 0 ? 1 : 0);
+    allocation_size += image_height;
+    if (allocation_size.has_overflow())
+        return Error::from_string_literal("CCITTDecoder: image dimensions too large");
+    ByteBuffer decoded_bytes = TRY(ByteBuffer::create_zeroed(allocation_size.value()));
     auto output_stream = make<FixedMemoryStream>(decoded_bytes.bytes());
     auto decoded_bits = make<BigEndianOutputBitStream>(MaybeOwned<Stream>(*output_stream));
 

Userland/Libraries/LibGfx/ImageFormats/TIFFLoader.cpp

@@ -5,6 +5,7 @@
  */
 
 #include "TIFFLoader.h"
+#include <AK/Checked.h>
 #include <AK/ConstrainedStream.h>
 #include <AK/Debug.h>
 #include <AK/Endian.h>
@@ -384,7 +385,20 @@ class TIFFLoadingContext {
         for (u32 segment_index = 0; segment_index < offsets.size(); ++segment_index) {
             TRY(m_stream->seek(offsets[segment_index]));
 
-            auto const rows_in_segment = segment_index < offsets.size() - 1 ? segment_length : *m_metadata.image_length() - segment_length * segment_index;
+            // For tiled images, segment_index is a linear index across all tile columns then rows.
+            // The tile row is segment_index / segment_per_rows; all tiles in the same tile row
+            // start at the same image y-offset. Using segment_index directly would give a false
+            // overflow for valid tiled TIFFs with multiple tile columns.
+            u32 const tile_row = segment_index / segment_per_rows;
+
+            // Validate that rows already decoded by prior tile rows don't exceed image_length,
+            // which would cause unsigned underflow in the last-segment row calculation below.
+            Checked<u32> rows_used_before_this_tile_row = segment_length;
+            rows_used_before_this_tile_row *= tile_row;
+            if (rows_used_before_this_tile_row.has_overflow() || rows_used_before_this_tile_row.value() > *m_metadata.image_length())
+                return Error::from_string_literal("TIFFImageDecoderPlugin: Invalid strip/tile configuration: too many segments for image height");
+
+            auto const rows_in_segment = segment_index < offsets.size() - 1 ? segment_length : *m_metadata.image_length() - rows_used_before_this_tile_row.value();
             auto decoded_bytes = TRY(segment_decoder(byte_counts[segment_index], { segment_width, rows_in_segment }));
 
             ByteBuffer differential_predictor_buffer;
@@ -397,15 +411,21 @@ class TIFFLoadingContext {
             auto decoded_stream = make<BigEndianInputBitStream>(move(decoded_segment));
 
             for (u32 row = 0; row < segment_length; row++) {
-                auto const image_row = row + segment_length * (segment_index / segment_per_rows);
-                if (image_row >= *m_metadata.image_length())
+                Checked<u32> checked_image_row = segment_length;
+                checked_image_row *= tile_row;
+                checked_image_row += row;
+                if (checked_image_row.has_overflow() || checked_image_row.value() >= *m_metadata.image_length())
                     break;
+                auto const image_row = checked_image_row.value();
 
                 for (u32 column = 0; column < segment_width; ++column) {
                     // If image_length % segment_length != 0, the last tile will be padded.
                     // This variable helps us to skip these last columns. Note that we still
                     // need to read the sample from the stream.
-                    auto const image_column = column + segment_width * (segment_index % segment_per_rows);
+                    Checked<u32> checked_image_column = segment_width;
+                    checked_image_column *= (segment_index % segment_per_rows);
+                    checked_image_column += column;
+                    auto const image_column = checked_image_column.has_overflow() ? m_image_width : checked_image_column.value();
 
                     if (m_photometric_interpretation == PhotometricInterpretation::CMYK) {
                         auto const cmyk = TRY(read_color_cmyk(*decoded_stream));