-
Notifications
You must be signed in to change notification settings - Fork 15k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 013961609785 from chromium
- Loading branch information
1 parent
dc8a39c
commit fa779b9
Showing
2 changed files
with
141 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
From 0139616097859b369120628ea843598550929526 Mon Sep 17 00:00:00 2001 | ||
From: John Stiles <johnstiles@google.com> | ||
Date: Thu, 28 Mar 2024 00:51:13 +0000 | ||
Subject: [PATCH] Detect overflow in JPEG image size calculations. | ||
|
||
Bug: 330756841 | ||
Change-Id: Ib30493152e08fd2347f76de276c5805d6fef9a7d | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5402199 | ||
Commit-Queue: John Stiles <johnstiles@google.com> | ||
Reviewed-by: Daniel Cheng <dcheng@chromium.org> | ||
Cr-Commit-Position: refs/heads/main@{#1279376} | ||
--- | ||
|
||
diff --git a/ui/gfx/codec/jpeg_codec.cc b/ui/gfx/codec/jpeg_codec.cc | ||
index a4de170..595966d 100644 | ||
--- a/ui/gfx/codec/jpeg_codec.cc | ||
+++ b/ui/gfx/codec/jpeg_codec.cc | ||
@@ -6,10 +6,12 @@ | ||
|
||
#include <setjmp.h> | ||
|
||
+#include <climits> | ||
#include <memory> | ||
#include <ostream> | ||
|
||
#include "base/notreached.h" | ||
+#include "base/numerics/checked_math.h" | ||
#include "third_party/skia/include/core/SkBitmap.h" | ||
#include "third_party/skia/include/core/SkColorPriv.h" | ||
#include "ui/gfx/codec/vector_wstream.h" | ||
@@ -244,16 +246,27 @@ | ||
|
||
jpeg_start_decompress(cinfo.get()); | ||
|
||
- // FIXME(brettw) we may want to allow the capability for callers to request | ||
- // how to align row lengths as we do for the compressor. | ||
- int row_read_stride = cinfo->output_width * cinfo->output_components; | ||
+ // Confirm that the image width * height * component-size fits within an int. | ||
+ // Note that image width and height are unsigned ints (JDIMENSION) in memory, | ||
+ // but the file format only holds a uint16. | ||
+ base::CheckedNumeric<size_t> checked_output_size = cinfo->output_width; | ||
+ checked_output_size *= cinfo->output_components; | ||
|
||
- // Create memory for a decoded image and write decoded lines to the memory | ||
- // without conversions same as JPEGCodec::Encode(). | ||
- int row_write_stride = row_read_stride; | ||
- output->resize(row_write_stride * cinfo->output_height); | ||
+ // This shouldn't ever overflow a `size_t`; it's multiplying a uint16 by four. | ||
+ size_t row_write_stride = checked_output_size.ValueOrDie(); | ||
|
||
- for (int row = 0; row < static_cast<int>(cinfo->output_height); row++) { | ||
+ // Extremely large JPEGs could overflow here if `size_t` is 32 bits. | ||
+ checked_output_size *= cinfo->output_height; | ||
+ size_t output_size = checked_output_size.ValueOrDefault(INT_MAX); | ||
+ if (output_size >= INT_MAX) { | ||
+ return false; | ||
+ } | ||
+ | ||
+ // Create memory for a decoded image. | ||
+ output->resize(output_size); | ||
+ | ||
+ // Write decoded lines to the memory. | ||
+ for (unsigned int row = 0; row < cinfo->output_height; row++) { | ||
unsigned char* rowptr = &(*output)[row * row_write_stride]; | ||
if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1)) | ||
return false; | ||
diff --git a/ui/gfx/codec/jpeg_codec_unittest.cc b/ui/gfx/codec/jpeg_codec_unittest.cc | ||
index 9f1bee9..b446fe7 100644 | ||
--- a/ui/gfx/codec/jpeg_codec_unittest.cc | ||
+++ b/ui/gfx/codec/jpeg_codec_unittest.cc | ||
@@ -61,6 +61,53 @@ | ||
"\xfa\xff\xda\x00\x0c\x03\x01\x00\x02\x11\x03\x11\x00\x3f\x00\xf9" | ||
"\xd2\x8a\x28\xaf\xc3\x0f\xf5\x4c\xff\xd9"; | ||
|
||
+// This is a copy of the above JPEG, with the Start-of-Frame header manually | ||
+// rewritten to indicate an image size of 25000x25000. An image this large would | ||
+// require more than 2GB of RAM to decode, so the decoder will reject the image | ||
+// as soon as the header is parsed. | ||
+const uint8_t kExtremelyLargeTestImage[] = | ||
+ "\xff\xd8\xff\xe0\x00\x10\x4a\x46\x49\x46\x00\x01\x01\x00\x00\x01" | ||
+ "\x00\x01\x00\x00\xff\xdb\x00\x43\x00\x03\x02\x02\x03\x02\x02\x03" | ||
+ "\x03\x03\x03\x04\x03\x03\x04\x05\x08\x05\x05\x04\x04\x05\x0a\x07" | ||
+ "\x07\x06\x08\x0c\x0a\x0c\x0c\x0b\x0a\x0b\x0b\x0d\x0e\x12\x10\x0d" | ||
+ "\x0e\x11\x0e\x0b\x0b\x10\x16\x10\x11\x13\x14\x15\x15\x15\x0c\x0f" | ||
+ "\x17\x18\x16\x14\x18\x12\x14\x15\x14\xff\xdb\x00\x43\x01\x03\x04" | ||
+ "\x04\x05\x04\x05\x09\x05\x05\x09\x14\x0d\x0b\x0d\x14\x14\x14\x14" | ||
+ "\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14" | ||
+ "\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14" | ||
+ "\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\x14\xff\xc0" | ||
+ "\x00\x11\x08\x61\xa8\x61\xa8\x03\x01\x22\x00\x02\x11\x01\x03\x11" | ||
+ // ^^ ^^ ^^ ^^ image size forced to 25000x25000 | ||
+ "\x01\xff\xc4\x00\x1f\x00\x00\x01\x05\x01\x01\x01\x01\x01\x01\x00" | ||
+ "\x00\x00\x00\x00\x00\x00\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09" | ||
+ "\x0a\x0b\xff\xc4\x00\xb5\x10\x00\x02\x01\x03\x03\x02\x04\x03\x05" | ||
+ "\x05\x04\x04\x00\x00\x01\x7d\x01\x02\x03\x00\x04\x11\x05\x12\x21" | ||
+ "\x31\x41\x06\x13\x51\x61\x07\x22\x71\x14\x32\x81\x91\xa1\x08\x23" | ||
+ "\x42\xb1\xc1\x15\x52\xd1\xf0\x24\x33\x62\x72\x82\x09\x0a\x16\x17" | ||
+ "\x18\x19\x1a\x25\x26\x27\x28\x29\x2a\x34\x35\x36\x37\x38\x39\x3a" | ||
+ "\x43\x44\x45\x46\x47\x48\x49\x4a\x53\x54\x55\x56\x57\x58\x59\x5a" | ||
+ "\x63\x64\x65\x66\x67\x68\x69\x6a\x73\x74\x75\x76\x77\x78\x79\x7a" | ||
+ "\x83\x84\x85\x86\x87\x88\x89\x8a\x92\x93\x94\x95\x96\x97\x98\x99" | ||
+ "\x9a\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xb2\xb3\xb4\xb5\xb6\xb7" | ||
+ "\xb8\xb9\xba\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xd2\xd3\xd4\xd5" | ||
+ "\xd6\xd7\xd8\xd9\xda\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xf1" | ||
+ "\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xff\xc4\x00\x1f\x01\x00\x03" | ||
+ "\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x00\x00\x00\x00\x00\x01" | ||
+ "\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\xff\xc4\x00\xb5\x11\x00" | ||
+ "\x02\x01\x02\x04\x04\x03\x04\x07\x05\x04\x04\x00\x01\x02\x77\x00" | ||
+ "\x01\x02\x03\x11\x04\x05\x21\x31\x06\x12\x41\x51\x07\x61\x71\x13" | ||
+ "\x22\x32\x81\x08\x14\x42\x91\xa1\xb1\xc1\x09\x23\x33\x52\xf0\x15" | ||
+ "\x62\x72\xd1\x0a\x16\x24\x34\xe1\x25\xf1\x17\x18\x19\x1a\x26\x27" | ||
+ "\x28\x29\x2a\x35\x36\x37\x38\x39\x3a\x43\x44\x45\x46\x47\x48\x49" | ||
+ "\x4a\x53\x54\x55\x56\x57\x58\x59\x5a\x63\x64\x65\x66\x67\x68\x69" | ||
+ "\x6a\x73\x74\x75\x76\x77\x78\x79\x7a\x82\x83\x84\x85\x86\x87\x88" | ||
+ "\x89\x8a\x92\x93\x94\x95\x96\x97\x98\x99\x9a\xa2\xa3\xa4\xa5\xa6" | ||
+ "\xa7\xa8\xa9\xaa\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xc2\xc3\xc4" | ||
+ "\xc5\xc6\xc7\xc8\xc9\xca\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xe2" | ||
+ "\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9" | ||
+ "\xfa\xff\xda\x00\x0c\x03\x01\x00\x02\x11\x03\x11\x00\x3f\x00\xf9" | ||
+ "\xd2\x8a\x28\xaf\xc3\x0f\xf5\x4c\xff\xd9"; | ||
+ | ||
} // namespace | ||
|
||
namespace gfx { | ||
@@ -211,4 +258,15 @@ | ||
encode_loop.Run(); | ||
} | ||
|
||
+TEST(JPEGCodec, ExtremelyLargeImage) { | ||
+ std::vector<unsigned char> output; | ||
+ int outw, outh; | ||
+ bool ok = JPEGCodec::Decode(kExtremelyLargeTestImage, | ||
+ std::size(kExtremelyLargeTestImage), | ||
+ JPEGCodec::FORMAT_RGBA, &output, &outw, &outh); | ||
+ EXPECT_FALSE(ok); | ||
+ EXPECT_EQ(outw, 25000); | ||
+ EXPECT_EQ(outh, 25000); | ||
+} | ||
+ | ||
} // namespace gfx |