Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 013961609785 from chromium #42069

Merged
merged 3 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,5 @@ bindings_refactor_domdatastore.patch
merge_fix_domarraybuffer_isdetached_and_comment_out_a_check.patch
cherry-pick-98bcf9ef5cdd.patch
cherry-pick-c1f25647c2fc.patch
cherry-pick-013961609785.patch
a11y_avoid_clearing_resetting_focus_on_an_already_focus_event.patch
139 changes: 139 additions & 0 deletions patches/chromium/cherry-pick-013961609785.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: John Stiles <johnstiles@google.com>
Date: Thu, 28 Mar 2024 00:51:13 +0000
Subject: 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 a4de17094e69b36bb2c34b3b4f70eece9c088cc3..595966d724146df33701b1225e659ec085ca115b 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 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size,

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 9f1bee95e0e476b34382aceffbf6df3bdde095ea..b446fe77896e254820db77afe7ed4e48b8525c79 100644
--- a/ui/gfx/codec/jpeg_codec_unittest.cc
+++ b/ui/gfx/codec/jpeg_codec_unittest.cc
@@ -61,6 +61,53 @@ const uint8_t kTopSitesMigrationTestImage[] =
"\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 @@ TEST(JPEGCodec, ParallelEncoding) {
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