Extensions - Check for too big or too small manifest icons.

both product and browser action icons are affected by this check

BUG=none

Review URL: https://codereview.chromium.org/1618073002

Cr-Commit-Position: refs/heads/master@{#371311}

5 files changed, 80 insertions, 2 deletions

diff --git a/extensions/common/manifest_constants.cc b/extensions/common/manifest_constants.cc
index d948170..7195dda 100644
--- a/extensions/common/manifest_constants.cc
+++ b/extensions/common/manifest_constants.cc
@@ -406,6 +406,7 @@ const char kInvalidHomepageOverrideURL[] =
     "Invalid value for overriding homepage url: '[*]'.";
 const char kInvalidHomepageURL[] =
     "Invalid value for homepage url: '[*]'.";
+const char kInvalidIconKey[] = "Invalid key in icons: \"*\".";
 const char kInvalidIconPath[] =
     "Invalid value for 'icons[\"*\"]'.";
 const char kInvalidIcons[] =
diff --git a/extensions/common/manifest_constants.h b/extensions/common/manifest_constants.h
index b6978a9..9051aa8 100644
--- a/extensions/common/manifest_constants.h
+++ b/extensions/common/manifest_constants.h
@@ -325,6 +325,7 @@ extern const char kInvalidGlob[];
 extern const char kInvalidGlobList[];
 extern const char kInvalidHomepageOverrideURL[];
 extern const char kInvalidHomepageURL[];
+extern const char kInvalidIconKey[];
 extern const char kInvalidIconPath[];
 extern const char kInvalidIcons[];
 extern const char kInvalidImport[];
diff --git a/extensions/common/manifest_handler_helpers.cc b/extensions/common/manifest_handler_helpers.cc
index bfc9e33..4a644b8 100644
--- a/extensions/common/manifest_handler_helpers.cc
+++ b/extensions/common/manifest_handler_helpers.cc
@@ -10,6 +10,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_icon_set.h"
@@ -41,8 +42,13 @@ bool LoadIconsFromDictionary(const base::DictionaryValue* icons_value,
        !iterator.IsAtEnd(); iterator.Advance()) {
     int size = 0;
     std::string icon_path;
-    if (!base::StringToInt(iterator.key(), &size) ||
-        !iterator.value().GetAsString(&icon_path) ||
+    if (!base::StringToInt(iterator.key(), &size) || size <= 0 ||
+        size > extension_misc::EXTENSION_ICON_GIGANTOR * 4) {
+      *error = ErrorUtils::FormatErrorMessageUTF16(errors::kInvalidIconKey,
+                                                   iterator.key());
+      return false;
+    }
+    if (!iterator.value().GetAsString(&icon_path) ||
         !NormalizeAndValidatePath(&icon_path)) {
       *error = ErrorUtils::FormatErrorMessageUTF16(errors::kInvalidIconPath,
                                                    iterator.key());
diff --git a/extensions/common/manifest_handlers/icons_handler_unittest.cc b/extensions/common/manifest_handlers/icons_handler_unittest.cc
new file mode 100644
index 0000000..fa644a9
--- /dev/null
+++ b/extensions/common/manifest_handlers/icons_handler_unittest.cc
@@ -0,0 +1,69 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/test/values_test_util.h"
+#include "extensions/common/manifest_handlers/icons_handler.h"
+#include "extensions/common/manifest_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+class ProductIconManifestTest : public ManifestTest {
+ public:
+  ProductIconManifestTest() {}
+
+ protected:
+  scoped_ptr<base::DictionaryValue> CreateManifest(
+      const std::string& extra_icons) {
+    scoped_ptr<base::DictionaryValue> manifest = base::DictionaryValue::From(
+        base::test::ParseJson("{ \n"
+                              "  \"name\": \"test\", \n"
+                              "  \"version\": \"0.1\", \n"
+                              "  \"manifest_version\": 2, \n"
+                              "  \"icons\": { \n" +
+                              extra_icons + "    \"16\": \"icon1.png\", \n"
+                                            "    \"32\": \"icon2.png\" \n"
+                                            "  } \n"
+                                            "} \n"));
+    EXPECT_TRUE(manifest);
+    return manifest;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ProductIconManifestTest);
+};
+
+TEST_F(ProductIconManifestTest, Sizes) {
+  // Too big.
+  {
+    scoped_ptr<base::DictionaryValue> ext_manifest =
+        CreateManifest("\"100000\": \"icon3.png\", \n");
+    ManifestData manifest(std::move(ext_manifest), "test");
+    LoadAndExpectError(manifest, "Invalid key in icons: \"100000\".");
+  }
+  // Too small.
+  {
+    scoped_ptr<base::DictionaryValue> ext_manifest =
+        CreateManifest("\"0\": \"icon3.png\", \n");
+    ManifestData manifest(std::move(ext_manifest), "test");
+    LoadAndExpectError(manifest, "Invalid key in icons: \"0\".");
+  }
+  // NaN.
+  {
+    scoped_ptr<base::DictionaryValue> ext_manifest =
+        CreateManifest("\"sixteen\": \"icon3.png\", \n");
+    ManifestData manifest(std::move(ext_manifest), "test");
+    LoadAndExpectError(manifest, "Invalid key in icons: \"sixteen\".");
+  }
+  // Just right.
+  {
+    scoped_ptr<base::DictionaryValue> ext_manifest =
+        CreateManifest("\"512\": \"icon3.png\", \n");
+    ManifestData manifest(std::move(ext_manifest), "test");
+    scoped_refptr<extensions::Extension> extension =
+        LoadAndExpectSuccess(manifest);
+  }
+}
+
+}  // namespace extensions
diff --git a/extensions/extensions_tests.gypi b/extensions/extensions_tests.gypi
index 2c7209a..b226110 100644
--- a/extensions/extensions_tests.gypi
+++ b/extensions/extensions_tests.gypi
@@ -126,6 +126,7 @@
       'common/manifest_handlers/default_locale_manifest_unittest.cc',
       'common/manifest_handlers/externally_connectable_unittest.cc',
       'common/manifest_handlers/file_handler_manifest_unittest.cc',
+      'common/manifest_handlers/icons_handler_unittest.cc',
       'common/manifest_handlers/incognito_manifest_unittest.cc',
       'common/manifest_handlers/kiosk_mode_info_unittest.cc',
       'common/manifest_handlers/oauth2_manifest_unittest.cc',