Data decoder: Limit JPEG decode size

This caps JPEG decode size in the Data Decoder service at 1 GB of
uncompressed image data in an attempt to stop excessively large image
decodes from inducing OOM crashes.

Bug: 1014022
Change-Id: Idb3dfed8cf383fbe58d9521093a1d11f7feee01e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866208Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#709170}

services/data_decoder/image_decoder_impl.cc

@@ -25,6 +25,13 @@ namespace data_decoder {
 
 namespace {
 
+#if defined(OS_CHROMEOS)
+// NOTE: This 1 GB limit is arbitrary and may be subject to change. The purpose
+// of limiting image decode size is to avoid OOM crashes caused by very large
+// image data being thrown at the service.
+constexpr size_t kJpegMaxDecodedNumBytes = 1024 * 1024 * 1024;
+#endif
+
 int64_t kPadding = 64;
 
 void ResizeImage(SkBitmap* decoded_image,
@@ -81,7 +88,7 @@ void ImageDecoderImpl::DecodeImage(const std::vector<uint8_t>& encoded_data,
     // Our robust jpeg decoding is using IJG libjpeg.
     if (encoded_data.size()) {
       std::unique_ptr<SkBitmap> decoded_jpeg(gfx::JPEGCodecRobustSlow::Decode(
-          encoded_data.data(), encoded_data.size()));
+          encoded_data, kJpegMaxDecodedNumBytes));
       if (decoded_jpeg.get() && !decoded_jpeg->empty())
         decoded_image = *decoded_jpeg;
     }

ui/gfx/codec/chromeos/jpeg_codec_robust_slow.cc

@@ -9,6 +9,7 @@
 #include <memory>
 
 #include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
 
 extern "C" {
 // IJG provides robust JPEG decode
@@ -43,14 +44,6 @@ void IjgErrorExit(jpeg_common_struct* cinfo) {
 
 namespace {
 
-struct IjgJpegDecoderState {
-  IjgJpegDecoderState(const unsigned char* in, size_t len)
-      : input_buffer(in), input_buffer_length(len) {}
-
-  const unsigned char* input_buffer;
-  size_t input_buffer_length;
-};
-
 // Callback to initialize the source.
 //
 // From the JPEG library:
@@ -58,10 +51,10 @@ struct IjgJpegDecoderState {
 //   actually read. May leave bytes_in_buffer set to 0 (in which case a
 //   fill_input_buffer() call will occur immediately)."
 void IjgInitSource(j_decompress_ptr cinfo) {
-  IjgJpegDecoderState* state =
-      static_cast<IjgJpegDecoderState*>(cinfo->client_data);
-  cinfo->src->next_input_byte = state->input_buffer;
-  cinfo->src->bytes_in_buffer = state->input_buffer_length;
+  auto* compressed_data =
+      static_cast<base::span<const uint8_t>*>(cinfo->client_data);
+  cinfo->src->next_input_byte = compressed_data->data();
+  cinfo->src->bytes_in_buffer = compressed_data->size();
 }
 
 // Callback to fill the buffer. Since our buffer already contains all the data,
@@ -118,7 +111,7 @@ void IjgTermSource(j_decompress_ptr cinfo) {}
 #if !defined(JCS_EXTENSIONS)
 // Converts one row of rgb data to rgba data by adding a fully-opaque alpha
 // value.
-void AddAlpha(const unsigned char* rgb, int pixel_width, unsigned char* rgba) {
+void AddAlpha(const uint8_t* rgb, int pixel_width, uint8_t* rgba) {
   for (int x = 0; x < pixel_width; x++) {
     memcpy(&rgba[x * 4], &rgb[x * 3], 3);
     rgba[x * 4 + 3] = 0xff;
@@ -127,10 +120,10 @@ void AddAlpha(const unsigned char* rgb, int pixel_width, unsigned char* rgba) {
 
 // Converts one row of RGB data to BGRA by reordering the color components and
 // adding alpha values of 0xff.
-void RGBtoBGRA(const unsigned char* bgra, int pixel_width, unsigned char* rgb) {
+void RGBtoBGRA(const uint8_t* bgra, int pixel_width, uint8_t* rgb) {
   for (int x = 0; x < pixel_width; x++) {
-    const unsigned char* pixel_in = &bgra[x * 3];
-    unsigned char* pixel_out = &rgb[x * 4];
+    const uint8_t* pixel_in = &bgra[x * 3];
+    uint8_t* pixel_out = &rgb[x * 4];
     pixel_out[0] = pixel_in[2];
     pixel_out[1] = pixel_in[1];
     pixel_out[2] = pixel_in[0];
@@ -149,12 +142,12 @@ struct JpegRobustDecompressStructDeleter {
 
 }  // namespace
 
-bool JPEGCodecRobustSlow::Decode(const unsigned char* input,
-                                 size_t input_size,
+bool JPEGCodecRobustSlow::Decode(base::span<const uint8_t> compressed_data,
                                  ColorFormat format,
-                                 std::vector<unsigned char>* output,
+                                 std::vector<uint8_t>* output,
                                  int* w,
-                                 int* h) {
+                                 int* h,
+                                 base::Optional<size_t> max_decoded_num_bytes) {
   std::unique_ptr<jpeg_decompress_struct, JpegRobustDecompressStructDeleter>
       cinfo(new jpeg_decompress_struct);
   output->clear();
@@ -185,8 +178,7 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input,
   srcmgr.term_source = IjgTermSource;
   cinfo->src = &srcmgr;
 
-  IjgJpegDecoderState state(input, input_size);
-  cinfo->client_data = &state;
+  cinfo->client_data = &compressed_data;
 
   // fill the file metadata into our buffer
   if (jpeg_read_header(cinfo.get(), true) != JPEG_HEADER_OK)
@@ -237,83 +229,83 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input,
   *w = cinfo->output_width;
   *h = cinfo->output_height;
 
-  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;
-
-#ifdef JCS_EXTENSIONS
-  // 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);
-
-  for (int row = 0; row < static_cast<int>(cinfo->output_height); row++) {
-    unsigned char* rowptr = &(*output)[row * row_write_stride];
-    if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1))
-      return false;
-  }
+  const size_t decoded_row_stride =
+      cinfo->output_width *
+      base::saturated_cast<size_t>(cinfo->output_components);
+  void (*converter)(const uint8_t* rgb, int w, uint8_t* out) = nullptr;
+  size_t output_row_stride;
+
+#if defined(JCS_EXTENSIONS)
+  // Easy case: rows need no pixel format conversion.
+  output_row_stride = decoded_row_stride;
 #else
   if (format == FORMAT_RGB) {
-    // easy case, row needs no conversion
-    int row_write_stride = row_read_stride;
-    output->resize(row_write_stride * cinfo->output_height);
-
-    for (int row = 0; row < static_cast<int>(cinfo->output_height); row++) {
-      unsigned char* rowptr = &(*output)[row * row_write_stride];
-      if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1))
-        return false;
-    }
+    // Easy case: rows need no pixel format conversion.
+    output_row_stride = decoded_row_stride;
   } else {
-    // Rows need conversion to output format: read into a temporary buffer and
-    // expand to the final one. Performance: we could avoid the extra
-    // allocation by doing the expansion in-place.
-    int row_write_stride;
-    void (*converter)(const unsigned char* rgb, int w, unsigned char* out);
+    // Rows will need a pixel format conversion to output format.
     if (format == FORMAT_RGBA ||
         (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
-      row_write_stride = cinfo->output_width * 4;
+      output_row_stride = cinfo->output_width * 4;
       converter = AddAlpha;
     } else if (format == FORMAT_BGRA ||
                (format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) {
-      row_write_stride = cinfo->output_width * 4;
+      output_row_stride = cinfo->output_width * 4;
       converter = RGBtoBGRA;
     } else {
       NOTREACHED() << "Invalid pixel format";
-      jpeg_destroy_decompress(cinfo.get());
       return false;
     }
+  }
+#endif
 
-    output->resize(row_write_stride * cinfo->output_height);
+  const size_t output_num_bytes = output_row_stride * cinfo->output_height;
+  if (max_decoded_num_bytes && output_num_bytes > *max_decoded_num_bytes)
+    return false;
+
+  jpeg_start_decompress(cinfo.get());
 
-    std::unique_ptr<unsigned char[]> row_data(
-        new unsigned char[row_read_stride]);
-    unsigned char* rowptr = row_data.get();
-    for (int row = 0; row < static_cast<int>(cinfo->output_height); row++) {
+  output->resize(output_num_bytes);
+  if (converter) {
+    // The decoded pixels need to be converted to the output format. We reuse
+    // this temporary buffer to decode each row and then write their converted
+    // form to the real output buffer.
+    auto row_data = std::make_unique<uint8_t[]>(decoded_row_stride);
+    uint8_t* rowptr = row_data.get();
+    for (size_t row = 0; row < cinfo->output_height; ++row) {
+      if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1))
+        return false;
+      converter(rowptr, *w, &(*output)[row * output_row_stride]);
+    }
+  } else {
+    for (size_t row = 0; row < cinfo->output_height; ++row) {
+      uint8_t* rowptr = &(*output)[row * output_row_stride];
       if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1))
         return false;
-      converter(rowptr, *w, &(*output)[row * row_write_stride]);
     }
   }
-#endif
 
   jpeg_finish_decompress(cinfo.get());
   return true;
 }
 
 // static
-SkBitmap* JPEGCodecRobustSlow::Decode(const unsigned char* input,
-                                      size_t input_size) {
+std::unique_ptr<SkBitmap> JPEGCodecRobustSlow::Decode(
+    base::span<const uint8_t> compressed_data,
+    base::Optional<size_t> max_decoded_num_bytes) {
   int w, h;
-  std::vector<unsigned char> data_vector;
-  if (!Decode(input, input_size, FORMAT_SkBitmap, &data_vector, &w, &h))
-    return NULL;
+  std::vector<uint8_t> data_vector;
+  if (!Decode(compressed_data, FORMAT_SkBitmap, &data_vector, &w, &h,
+              max_decoded_num_bytes)) {
+    return nullptr;
+  }
 
   // Skia only handles 32 bit images.
   int data_length = w * h * 4;
 
-  SkBitmap* bitmap = new SkBitmap();
+  auto bitmap = std::make_unique<SkBitmap>();
   bitmap->allocN32Pixels(w, h);
   memcpy(bitmap->getAddr32(0, 0), &data_vector[0], data_length);
 

ui/gfx/codec/chromeos/jpeg_codec_robust_slow.h

@@ -8,6 +8,8 @@
 #include <stddef.h>
 #include <vector>
 
+#include "base/containers/span.h"
+#include "base/optional.h"
 #include "ui/gfx/codec/codec_export.h"
 
 class SkBitmap;
@@ -37,21 +39,26 @@ class CODEC_EXPORT JPEGCodecRobustSlow {
     FORMAT_SkBitmap
   };
 
-  // Decodes the JPEG data contained in input of length input_size. The
-  // decoded data will be placed in *output with the dimensions in *w and *h
-  // on success (returns true). This data will be written in the'format'
-  // format. On failure, the values of these output variables is undefined.
-  static bool Decode(const unsigned char* input,
-                     size_t input_size,
+  // Decodes the JPEG data contained in |compressed_data|. The decoded data will
+  // be placed in |*output| with the dimensions in |*w| and |*h| on success
+  // (returns |true|). This data will be written in the |format| format.
+  //
+  // On failure, the values of these output variables is undefined.
+  //
+  // This will fail immediately without attempting to decode aything if the
+  // decompressed image data size would exceed |max_decoded_num_bytes| when
+  // given.
+  static bool Decode(base::span<const uint8_t> compressed_data,
                      ColorFormat format,
-                     std::vector<unsigned char>* output,
+                     std::vector<uint8_t>* output,
                      int* w,
-                     int* h);
+                     int* h,
+                     base::Optional<size_t> max_decoded_num_bytes);
 
-  // Decodes the JPEG data contained in input of length input_size. If
-  // successful, a SkBitmap is created and returned. It is up to the caller
-  // to delete the returned bitmap.
-  static SkBitmap* Decode(const unsigned char* input, size_t input_size);
+  // Same as above, but the image is returned as an SkBitmap.
+  static std::unique_ptr<SkBitmap> Decode(
+      base::span<const uint8_t> compressed_data,
+      base::Optional<size_t> max_decoded_num_bytes);
 };
 
 }  // namespace gfx

ui/gfx/codec/chromeos/jpeg_codec_robust_slow_unittest.cc

@@ -5,6 +5,7 @@
 #include <math.h>
 #include <stdint.h>
 
+#include "base/containers/span.h"
 #include "base/stl_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/codec/chromeos/jpeg_codec_robust_slow.h"
@@ -59,7 +60,7 @@ const uint8_t kTopSitesMigrationTestImage[] =
 
 namespace gfx {
 
-static void MakeRGBImage(int w, int h, std::vector<unsigned char>* dat) {
+static void MakeRGBImage(int w, int h, std::vector<uint8_t>* dat) {
   dat->resize(w * h * 3);
   for (int y = 0; y < h; y++) {
     for (int x = 0; x < w; x++) {
@@ -76,15 +77,15 @@ TEST(JPEGCodecRobustSlow, DecodeCorrupted) {
   int w = 20, h = 20;
 
   // some random data (an uncompressed image)
-  std::vector<unsigned char> original;
+  std::vector<uint8_t> original;
   MakeRGBImage(w, h, &original);
 
   // it should fail when given non-JPEG compressed data
-  std::vector<unsigned char> output;
+  std::vector<uint8_t> output;
   int outw, outh;
-  ASSERT_FALSE(JPEGCodecRobustSlow::Decode(&original[0], original.size(),
-                                           JPEGCodecRobustSlow::FORMAT_RGB,
-                                           &output, &outw, &outh));
+  ASSERT_FALSE(JPEGCodecRobustSlow::Decode(
+      original, JPEGCodecRobustSlow::FORMAT_RGB, &output, &outw, &outh,
+      base::nullopt /* max_decoded_num_bytes */));
 }
 
 // Test that we can decode JPEG images without invalid-read errors on valgrind.
@@ -94,11 +95,20 @@ TEST(JPEGCodecRobustSlow, InvalidRead) {
   std::vector<unsigned char> output;
   int outw, outh;
   ASSERT_TRUE(JPEGCodecRobustSlow::Decode(
-      kTopSitesMigrationTestImage, base::size(kTopSitesMigrationTestImage),
-      JPEGCodecRobustSlow::FORMAT_RGB, &output, &outw, &outh));
+      kTopSitesMigrationTestImage, JPEGCodecRobustSlow::FORMAT_RGB, &output,
+      &outw, &outh, base::nullopt /* max_decoded_num_bytes */));
   ASSERT_TRUE(JPEGCodecRobustSlow::Decode(
-      kTopSitesMigrationTestImage, base::size(kTopSitesMigrationTestImage),
-      JPEGCodecRobustSlow::FORMAT_RGBA, &output, &outw, &outh));
+      kTopSitesMigrationTestImage, JPEGCodecRobustSlow::FORMAT_RGBA, &output,
+      &outw, &outh, base::nullopt /* max_decoded_num_bytes */));
+}
+
+// Test that we can limit the size of a decode operation.
+TEST(JPEGCodecRobustSlow, MaxDecodedSize) {
+  std::vector<uint8_t> output;
+  int outw, outh;
+  EXPECT_FALSE(JPEGCodecRobustSlow::Decode(
+      kTopSitesMigrationTestImage, JPEGCodecRobustSlow::FORMAT_RGB, &output,
+      &outw, &outh, 1 /* max_decoded_num_bytes */));
 }
 
 }  // namespace gfx