In the APNG decoder, validate the chunk data length before calling GetChunkSize to avoid potential overflow in the chunk size calculation (#187949)

Before this PR, APNGImageGenerator::IsValidChunkHeader was calling
GetChunkSize to check whether the buffer had sufficient capacity for the
chunk.
The chunk contains a 32-bit data length field, and GetChunkSize
calculates the chunk size as a size_t. If size_t is 32-bit and the chunk
data length is malformed, then the calculation could overflow and return
an incorrect result.

This PR verifies that the chunk's data length fits within the remaining
capacity of the buffer before using the length in calculations.

See https://github.com/flutter/flutter/pull/187701

---------

Co-authored-by: Himanshu Anand <anand.himanshu17@gmail.com>

engine/src/flutter/lib/ui/painting/image_generator_apng.cc

diff --git a/engine/src/flutter/lib/ui/painting/image_generator_apng.cc b/engine/src/flutter/lib/ui/painting/image_generator_apng.cc
index 307bb5b..dc646b5 100644
--- a/engine/src/flutter/lib/ui/painting/image_generator_apng.cc
+++ b/engine/src/flutter/lib/ui/painting/image_generator_apng.cc
@@ -325,23 +325,28 @@
 bool APNGImageGenerator::IsValidChunkHeader(const void* buffer,
                                             size_t size,
                                             const ChunkHeader* chunk) {
-  // Ensure the chunk doesn't start before the beginning of the buffer.
-  if (reinterpret_cast<const uint8_t*>(chunk) <
-      static_cast<const uint8_t*>(buffer)) {
+  // Ensure that the chunk starts within the bounds of the buffer.
+  const uint8_t* chunk_ptr = reinterpret_cast<const uint8_t*>(chunk);
+  const uint8_t* buffer_ptr = static_cast<const uint8_t*>(buffer);
+  if (chunk_ptr < buffer_ptr || chunk_ptr >= buffer_ptr + size) {
     return false;
   }
 
-  // Ensure the buffer is large enough to contain at least the chunk header.
-  if (reinterpret_cast<const uint8_t*>(chunk) + sizeof(ChunkHeader) >
-      static_cast<const uint8_t*>(buffer) + size) {
+  // Ensure that the buffer has enough space for the chunk header before using
+  // any fields in the header.
+  size_t buffer_bytes_remaining = size - (chunk_ptr - buffer_ptr);
+  if (buffer_bytes_remaining < sizeof(ChunkHeader)) {
     return false;
   }
-
-  // Ensure the buffer is large enough to contain the chunk's given data size
-  // and CRC.
-  const uint8_t* chunk_end =
-      reinterpret_cast<const uint8_t*>(chunk) + GetChunkSize(chunk);
-  if (chunk_end > static_cast<const uint8_t*>(buffer) + size) {
+  // Ensure that the buffer has enough space for the chunk data and CRC.
+  // Do not use the chunk data length in pointer arithmetic until it is known to
+  // be valid.
+  size_t data_length = chunk->get_data_length();
+  if (buffer_bytes_remaining - sizeof(ChunkHeader) < data_length) {
+    return false;
+  }
+  if (buffer_bytes_remaining - sizeof(ChunkHeader) - data_length <
+      kChunkCrcSize) {
     return false;
   }
 

engine/src/flutter/lib/ui/painting/image_generator_apng_unittests.cc

diff --git a/engine/src/flutter/lib/ui/painting/image_generator_apng_unittests.cc b/engine/src/flutter/lib/ui/painting/image_generator_apng_unittests.cc
index 283d07d..e626621 100644
--- a/engine/src/flutter/lib/ui/painting/image_generator_apng_unittests.cc
+++ b/engine/src/flutter/lib/ui/painting/image_generator_apng_unittests.cc
@@ -45,6 +45,18 @@
   WriteBE32(buf, crc);
 }
 
+// Appends a chunk with a declared data_length that may differ from the actual
+// data bytes written. Used to test handling of malformed chunks.
+void AppendChunkWithFakeLength(std::vector<uint8_t>& buf,
+                               const char type[4],
+                               uint32_t declared_length,
+                               const std::vector<uint8_t>& actual_data) {
+  WriteBE32(buf, declared_length);
+  buf.insert(buf.end(), type, type + 4);
+  buf.insert(buf.end(), actual_data.begin(), actual_data.end());
+  WriteBE32(buf, 0);  // CRC placeholder
+}
+
 // Builds a minimal valid APNG with a malicious fdAT chunk whose
 // data_length is less than 4, which would trigger an integer underflow
 // in DemuxNextImage() without the bounds check fix.
@@ -120,5 +132,53 @@
   EXPECT_NE(make_generator(4), nullptr);
 }
 
+TEST(APNGImageGeneratorTest, FdATWithOverflowDataLengthIsRejected) {
+  std::vector<uint8_t> apng(APNGImageGenerator::kPngSignature.begin(),
+                            APNGImageGenerator::kPngSignature.end());
+
+  // IHDR
+  {
+    std::vector<uint8_t> ihdr;
+    WriteBE32(ihdr, 1);
+    WriteBE32(ihdr, 1);
+    ihdr.push_back(8);
+    ihdr.push_back(6);
+    ihdr.push_back(0);
+    ihdr.push_back(0);
+    ihdr.push_back(0);
+    AppendChunk(apng, "IHDR", ihdr);
+  }
+  // acTL
+  {
+    std::vector<uint8_t> actl;
+    WriteBE32(actl, 1);
+    WriteBE32(actl, 0);
+    AppendChunk(apng, "acTL", actl);
+  }
+  // fcTL
+  {
+    std::vector<uint8_t> fctl;
+    WriteBE32(fctl, 0);
+    WriteBE32(fctl, 1);
+    WriteBE32(fctl, 1);
+    WriteBE32(fctl, 0);
+    WriteBE32(fctl, 0);
+    WriteBE16(fctl, 1);
+    WriteBE16(fctl, 10);
+    fctl.push_back(0);
+    fctl.push_back(0);
+    AppendChunk(apng, "fcTL", fctl);
+  }
+  // fdAT with declared data_length=0xFFFFFFFF but only 8 actual bytes
+  AppendChunkWithFakeLength(apng, "fdAT", 0xFFFFFFFF, {0, 0, 0, 1, 0, 0, 0, 0});
+  // IEND
+  AppendChunk(apng, "IEND", {});
+
+  auto data = SkData::MakeWithCopy(apng.data(), apng.size());
+  auto generator = APNGImageGenerator::MakeFromData(data);
+  // The generator should reject the malformed APNG without crashing.
+  EXPECT_EQ(generator, nullptr);
+}
+
 }  // namespace testing
 }  // namespace flutter