[Extensions Bindings] Protect against custom getters on chrome.runtime

Many chrome.extension methods are directly aliased to their
chrome.runtime counterparts. If the chrome.runtime object has a custom
getter that throws on access, trying to access the chrome.extension
variant will trigger it. Protect against this by checking the result
of the Get() in GetAliasedFeature().

Add regression tests in both a targeted unit test and an end-to-end
browser test.

Bug: 949170
Change-Id: I7405953a7ce735fe7d5fa055de8a5d97a1fbe1db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1553804
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648733}

chrome/browser/extensions/extension_bindings_apitest.cc

@@ -915,6 +915,57 @@ IN_PROC_BROWSER_TEST_F(ExtensionBindingsApiTest,
   EXPECT_EQ("success", result);
 }
 
+// Tests the aliasing of chrome.extension methods to their chrome.runtime
+// equivalents.
+IN_PROC_BROWSER_TEST_F(ExtensionBindingsApiTest,
+                       ChromeExtensionIsAliasedToChromeRuntime) {
+  constexpr char kManifest[] =
+      R"({
+           "name": "Test",
+           "version": "0.1",
+           "manifest_version": 2,
+           "background": { "scripts": ["background.js"] }
+         })";
+  constexpr char kBackground[] =
+      R"(chrome.test.runTests([
+           function chromeExtensionIsAliased() {
+             // Sanity check: chrome.extension is directly aliased to
+             // chrome.runtime.
+             chrome.test.assertTrue(!!chrome.runtime);
+             chrome.test.assertTrue(!!chrome.runtime.sendMessage);
+             chrome.test.assertEq(chrome.runtime.sendMessage,
+                                  chrome.extension.sendMessage);
+             chrome.test.succeed();
+           },
+           function testOverridingFailsGracefully() {
+             let intercepted = false;
+             // Modify the chrome.runtime object, which is the source for the
+             // chrome.extension API, to throw an error when sendMessage is
+             // accessed. Nothing should blow up.
+             // Regression test for https://crbug.com/949170.
+             Object.defineProperty(
+                 chrome.runtime,
+                 'sendMessage',
+                 {
+                   get() {
+                     intercepted = true;
+                     throw new Error('Mwahaha');
+                   }
+                 });
+             chrome.extension.sendMessage;
+             chrome.test.assertTrue(intercepted);
+             chrome.test.succeed();
+           }
+         ]);)";
+
+  TestExtensionDir extension_dir;
+  extension_dir.WriteManifest(kManifest);
+  extension_dir.WriteFile(FILE_PATH_LITERAL("background.js"), kBackground);
+  ResultCatcher catcher;
+  ASSERT_TRUE(LoadExtension(extension_dir.UnpackedPath()));
+  EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
+}
+
 INSTANTIATE_TEST_SUITE_P(,
                          ExtensionBindingsUserGestureTest,
                          ::testing::Values(kUserActivationV1,

chrome/renderer/extensions/extension_hooks_delegate.cc

@@ -77,8 +77,13 @@ void GetAliasedFeature(v8::Local<v8::Name> property_name,
   if (!has_property.IsJust() || !has_property.FromJust())
     return;
 
-  info.GetReturnValue().Set(
-      runtime_obj->Get(context, property_name).ToLocalChecked());
+  v8::Local<v8::Value> property_value;
+  // Try and grab the chrome.runtime version. It's possible this has been
+  // tampered with, so early-out if an exception is thrown.
+  if (!runtime_obj->Get(context, property_name).ToLocal(&property_value))
+    return;
+
+  info.GetReturnValue().Set(property_value);
 }
 
 // A helper method to throw a deprecation error on access.

chrome/renderer/extensions/extension_hooks_delegate_unittest.cc

@@ -212,4 +212,29 @@ TEST_F(ExtensionHooksDelegateTest, SendRequestChannelLeftOpenToReplyAsync) {
       messaging_service()->HasPortForTesting(script_context(), port_id));
 }
 
+// Tests that overriding the runtime equivalents of chrome.extension methods
+// with accessors that throw does not cause a crash on access. Regression test
+// for https://crbug.com/949170.
+TEST_F(ExtensionHooksDelegateTest, RuntimeAliasesCorrupted) {
+  v8::HandleScope handle_scope(isolate());
+  v8::Local<v8::Context> context = MainContext();
+
+  // Set a trap on chrome.runtime.sendMessage.
+  constexpr char kMutateChromeRuntime[] =
+      R"((function() {
+           Object.defineProperty(
+               chrome.runtime, 'sendMessage',
+               { get() { throw new Error('haha'); } });
+         }))";
+  RunFunctionOnGlobal(FunctionFromString(context, kMutateChromeRuntime),
+                      context, 0, nullptr);
+
+  // Touch chrome.extension.sendMessage, which is aliased to the runtime
+  // version. Though an error is thrown, we shouldn't crash.
+  constexpr char kTouchExtensionSendMessage[] =
+      "(function() { chrome.extension.sendMessage; })";
+  RunFunctionOnGlobal(FunctionFromString(context, kTouchExtensionSendMessage),
+                      context, 0, nullptr);
+}
+
 }  // namespace extensions