From ba21393f49c2118bd2d663fa72180a3b3f8abf53 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 5 Dec 2024 01:34:32 -0800 Subject: [PATCH] Drop APNG frames that don't fit entirely within the destination surface. (flutter/engine#56928) As per the [spec](https://www.w3.org/TR/png/#fcTL-chunk): > The frame must be rendered within the region defined by x_offset, y_offset, width, and height. This region may not fall outside of the default image; thus x_offset plus width must not be greater than the [IHDR](https://www.w3.org/TR/png/#11IHDR) width; similarly y_offset plus height must not be greater than the [IHDR](https://www.w3.org/TR/png/#11IHDR) height. --- .../lib/ui/fixtures/out_of_bounds.apng | Bin 0 -> 126 bytes .../painting/image_decoder_no_gl_unittests.cc | 2 +- .../lib/ui/painting/image_generator_apng.cc | 28 +++++++++++++++++- .../src/flutter/testing/dart/codec_test.dart | 20 +++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 engine/src/flutter/lib/ui/fixtures/out_of_bounds.apng diff --git a/engine/src/flutter/lib/ui/fixtures/out_of_bounds.apng b/engine/src/flutter/lib/ui/fixtures/out_of_bounds.apng new file mode 100644 index 0000000000000000000000000000000000000000..33993c7960e96e3d39b3d0b123ad3558ac9e6e6c GIT binary patch literal 126 zcmeAS@N?(olHy`uVBq!ia0vp^Od!m`1|*BN@u~nRj>O~;A0W*L!iTkPngMC4G>90G z3DE#z2!IGikh;U`H*o^l+@3CuAr*6y6CBtX7=#!Y%Rl}CDPiz*^>bP0l+XkKIP(-M literal 0 HcmV?d00001 diff --git a/engine/src/flutter/lib/ui/painting/image_decoder_no_gl_unittests.cc b/engine/src/flutter/lib/ui/painting/image_decoder_no_gl_unittests.cc index 04a0229c11..aad5032952 100644 --- a/engine/src/flutter/lib/ui/painting/image_decoder_no_gl_unittests.cc +++ b/engine/src/flutter/lib/ui/painting/image_decoder_no_gl_unittests.cc @@ -197,7 +197,7 @@ TEST(ImageDecoderNoGLTest, ImpellerWideGamutIndexedPng) { #endif // IMPELLER_SUPPORTS_RENDERING } -TEST(ImageDecoderNoGLTest, ImepllerUnmultipliedAlphaPng) { +TEST(ImageDecoderNoGLTest, ImpellerUnmultipliedAlphaPng) { #if defined(OS_FUCHSIA) GTEST_SKIP() << "Fuchsia can't load the test fixtures."; #endif 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 159d87638a..77b7b23767 100644 --- a/engine/src/flutter/lib/ui/painting/image_generator_apng.cc +++ b/engine/src/flutter/lib/ui/painting/image_generator_apng.cc @@ -110,6 +110,20 @@ bool APNGImageGenerator::GetPixels(const SkImageInfo& info, << ") of APNG due to the frame missing data (frame_info)."; return false; } + if (frame.x_offset + frame_info.width() > + static_cast(info.width()) || + frame.y_offset + frame_info.height() > + static_cast(info.height())) { + FML_DLOG(ERROR) + << "Decoded image at index " << image_index + << " (frame index: " << frame_index + << ") rejected because the destination region (x: " << frame.x_offset + << ", y: " << frame.y_offset << ", width: " << frame_info.width() + << ", height: " << frame_info.height() + << ") is not entirely within the destination surface (width: " + << info.width() << ", height: " << info.height() << ")."; + return false; + } //---------------------------------------------------------------------------- /// 3. Composite the frame onto the canvas. @@ -630,7 +644,19 @@ uint32_t APNGImageGenerator::ChunkHeader::ComputeChunkCrc32() { bool APNGImageGenerator::RenderDefaultImage(const SkImageInfo& info, void* pixels, size_t row_bytes) { - SkCodec::Result result = images_[0].codec->getPixels(info, pixels, row_bytes); + APNGImage& frame = images_[0]; + SkImageInfo frame_info = frame.codec->getInfo(); + if (frame_info.width() > info.width() || + frame_info.height() > info.height()) { + FML_DLOG(ERROR) + << "Default image rejected because the destination region (width: " + << frame_info.width() << ", height: " << frame_info.height() + << ") is not entirely within the destination surface (width: " + << info.width() << ", height: " << info.height() << ")."; + return false; + } + + SkCodec::Result result = frame.codec->getPixels(info, pixels, row_bytes); if (result != SkCodec::kSuccess) { FML_DLOG(ERROR) << "Failed to decode the APNG's default/fallback image. " "SkCodec::Result: " diff --git a/engine/src/flutter/testing/dart/codec_test.dart b/engine/src/flutter/testing/dart/codec_test.dart index e21b099a8c..197369c609 100644 --- a/engine/src/flutter/testing/dart/codec_test.dart +++ b/engine/src/flutter/testing/dart/codec_test.dart @@ -252,6 +252,26 @@ void main() { imageData = (await image.toByteData())!; expect(imageData.getUint32(imageData.lengthInBytes - 4), 0x00000000); }); + + test( + 'Animated apng frame decode does not crash with invalid destination region', + () async { + final Uint8List data = File( + path.join('flutter', 'lib', 'ui', 'fixtures', 'out_of_bounds.apng'), + ).readAsBytesSync(); + + final ui.Codec codec = await ui.instantiateImageCodec(data); + try { + await codec.getNextFrame(); + fail('exception not thrown'); + } on Exception catch (e) { + if (impellerEnabled) { + expect(e.toString(), contains('Could not decompress image.')); + } else { + expect(e.toString(), contains('Codec failed')); + } + } + }); } /// Returns a File handle to a file in the skia/resources directory.