Convert bitmaps received from the renderer to N32.

Although extensions now uses mojo to receive SkBitmaps, even those still
have a variety of color types. And some of those colortypes use other
pixel sizes than 32bpp, which can lead to buffer overflow bugs. So we
convert any incoming arbitrary bitmaps received from (extensions)
renderer processes to be N32 upon receipt.

R=reillyg@chromium.org

Bug: 1144462
Change-Id: I66acc7fbfcc0565411a1b9b9bc5a01b70558c5b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532663Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826425}

chrome/browser/extensions/api/declarative_content/content_action_unittest.cc

@@ -297,6 +297,49 @@ TEST(DeclarativeContentActionTest, SetIcon) {
   }
 }
 
+TEST(DeclarativeContentActionTest, SetInvalidIcon) {
+  TestExtensionEnvironment env;
+  content::RenderViewHostTestEnabler rvh_enabler;
+
+  // This bitmap type is allowed by skia::mojom::InlineBitmap but we should
+  // convert to N32 upon receipt.
+  SkBitmap bitmap;
+  EXPECT_TRUE(bitmap.tryAllocPixels(
+      SkImageInfo::Make(19, 19, kAlpha_8_SkColorType, kPremul_SkAlphaType)));
+  bitmap.eraseARGB(255, 255, 0, 0);
+
+  DictionaryBuilder builder;
+  builder.Set("instanceType", "declarativeContent.SetIcon");
+
+  std::vector<uint8_t> s = skia::mojom::InlineBitmap::Serialize(&bitmap);
+  builder.Set("imageData", DictionaryBuilder().Set("19", s).Build());
+
+  std::unique_ptr<base::DictionaryValue> dict = builder.Build();
+
+  const Extension* extension = env.MakeExtension(
+      ParseJson(R"({"page_action": {"default_title": "Extension"}})"));
+  base::HistogramTester histogram_tester;
+  TestingProfile profile;
+  std::string error;
+  std::unique_ptr<const ContentAction> result =
+      ContentAction::Create(&profile, extension, *dict, &error);
+  EXPECT_EQ("", error);
+  ASSERT_TRUE(result.get());
+
+  ExtensionAction* action = ExtensionActionManager::Get(env.profile())
+                                ->GetExtensionAction(*extension);
+
+  std::unique_ptr<content::WebContents> contents = env.MakeTab();
+  const int tab_id = ExtensionTabUtil::GetTabId(contents.get());
+  ContentAction::ApplyInfo apply_info = {extension, env.profile(),
+                                         contents.get(), 100};
+  EXPECT_TRUE(action->GetDeclarativeIcon(tab_id).IsEmpty());
+  result->Apply(apply_info);
+  EXPECT_FALSE(action->GetDeclarativeIcon(tab_id).IsEmpty());
+  EXPECT_EQ(kN32_SkColorType,
+            action->GetDeclarativeIcon(tab_id).ToSkBitmap()->colorType());
+}
+
 TEST(DeclarativeContentActionTest, SetInvisibleIcon) {
   TestExtensionEnvironment env;
 

extensions/browser/BUILD.gn

@@ -412,6 +412,8 @@ source_set("browser_sources") {
     "//services/metrics/public/cpp:metrics_cpp",
     "//services/preferences/public/cpp",
     "//services/service_manager/public/cpp",
+    "//skia",
+    "//skia/public/mojom",
     "//third_party/blink/public/common",
     "//third_party/blink/public/mojom/frame",
     "//ui/display",

extensions/browser/DEPS

@@ -37,7 +37,7 @@ include_rules = [
   "+services/network",
   "+services/preferences/public/cpp",
   "+services/service_manager/public/cpp",
-  "+skia/ext/image_operations.h",
+  "+skia/ext",
   "+third_party/leveldatabase",
   "+third_party/re2",
   "+third_party/blink/public/common",

extensions/browser/extension_action.cc

@@ -17,6 +17,7 @@
 #include "extensions/common/extension_icon_set.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "extensions/grit/extensions_browser_resources.h"
+#include "skia/ext/skia_utils_base.h"
 #include "skia/public/mojom/bitmap.mojom.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
@@ -136,10 +137,20 @@ ExtensionAction::IconParseResult ExtensionAction::ParseIconFromCanvasDictionary(
     } else {
       continue;
     }
+    SkBitmap unsafe_bitmap;
+    if (!skia::mojom::InlineBitmap::Deserialize(bytes, num_bytes,
+                                                &unsafe_bitmap)) {
+      return IconParseResult::kUnpickleFailure;
+    }
+    CHECK(!unsafe_bitmap.isNull());
+    // On receipt of an arbitrary bitmap from the renderer, we convert to an N32
+    // 32bpp bitmap. Other pixel sizes can lead to out-of-bounds mistakes when
+    // transferring the pixels out of the/ bitmap into other buffers.
     SkBitmap bitmap;
-    if (!skia::mojom::InlineBitmap::Deserialize(bytes, num_bytes, &bitmap))
+    if (!skia::SkBitmapToN32OpaqueOrPremul(unsafe_bitmap, &bitmap)) {
+      NOTREACHED() << "Unable to convert bitmap for icon";
       return IconParseResult::kUnpickleFailure;
-    CHECK(!bitmap.isNull());
+    }
 
     // Chrome helpfully scales the provided icon(s), but let's not go overboard.
     const int kActionIconMaxSize = 10 * ActionIconSize();