Extensions: Strengthen WebAccessibleResource checks

Currently a *compromised* web renderer can request any extension
resource from an extension that has web accessible resources (WAR). This
was necessary earlier since:

  - extension iframes didn't always live in the extension process when
  embedded in a website.

  - We needed to allow extension subresource requests from such iframes.
  These subresources didn't need to be web accessible.

  - This meant that the browser needed to allow cross renderer requests
  to such resources.

  - Hence as a compromise, at the browser level we allowlisted any cross
  renderer subresource request to an extension with WAR.

However, now extension iframes should always have their own process.
Hence the browser side check can be made more strict to disallow cross
renderer requests to non web accessible resources.

Note that we already block access to non web accessible extension
resources at the renderer level.

BUG=1093570, 179127, 173688.


Change-Id: I20523db2d1629f3714d3c84a27e0904ed4f1b27a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2244433
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Auto-Submit: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779744}

chrome/browser/apps/guest_view/web_view_browsertest.cc

@@ -3601,6 +3601,19 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, LoadWebviewInaccessibleResource) {
   EXPECT_EQ(foo_url, web_view_contents->GetLastCommittedURL());
 }
 
+// Ensure that only app resources accessible to the webview can be loaded in a
+// webview even if the webview commits an app frame.
+IN_PROC_BROWSER_TEST_F(WebViewTest,
+                       LoadAccessibleSubresourceInAppWebviewFrame) {
+  TestHelper("testLoadAccessibleSubresourceInAppWebviewFrame",
+             "web_view/load_webview_accessible_resource", NEEDS_TEST_SERVER);
+}
+IN_PROC_BROWSER_TEST_F(WebViewTest,
+                       InaccessibleResourceDoesNotLoadInAppWebviewFrame) {
+  TestHelper("testInaccessibleResourceDoesNotLoadInAppWebviewFrame",
+             "web_view/load_webview_accessible_resource", NEEDS_TEST_SERVER);
+}
+
 // Makes sure that a webview will display correctly after reloading it after a
 // crash.
 IN_PROC_BROWSER_TEST_F(WebViewTest, ReloadAfterCrash) {

chrome/test/data/extensions/platform_apps/web_view/load_webview_accessible_resource/assets/accessible.txt

+Hello World

chrome/test/data/extensions/platform_apps/web_view/load_webview_accessible_resource/assets/inaccessible.txt

+Hello World

chrome/test/data/extensions/platform_apps/web_view/load_webview_accessible_resource/embedder.js

@@ -14,6 +14,11 @@ window.runTest = function(testName) {
     testBlobInWebviewAccessibleResource();
   } else if (testName == 'testLoadWebviewInaccessibleResource') {
     testLoadWebviewInaccessibleResource();
+  } else if (testName == 'testLoadAccessibleSubresourceInAppWebviewFrame') {
+    testLoadAccessibleSubresourceInAppWebviewFrame();
+  } else if (
+      testName == 'testInaccessibleResourceDoesNotLoadInAppWebviewFrame') {
+    testInaccessibleResourceDoesNotLoadInAppWebviewFrame();
   } else if (testName == 'testNavigateGuestToWebviewAccessibleResource') {
     testNavigateGuestToWebviewAccessibleResource();
   } else {
@@ -55,8 +60,50 @@ function testNavigateGuestToWebviewAccessibleResource() {
         });
   });
 
-  webview.src =
-      'chrome-extension://lmnhajohhbenlbinimlfhbpnciehhmao/assets/foo.html';
+  webview.src = chrome.runtime.getURL('assets/foo.html');
+};
+
+function testInaccessibleResourceDoesNotLoadInAppWebviewFrame() {
+  var webview = document.querySelector('webview');
+
+  webview.addEventListener('loadstop', function() {
+    var script = `
+      fetch('inaccessible.txt')
+        .then(response => {
+          chrome.test.sendMessage('TEST_FAILED');
+        })
+        .catch(() => {
+          chrome.test.sendMessage('TEST_PASSED');
+        });
+    `;
+    webview.executeScript({code: script});
+  });
+
+  webview.src = chrome.runtime.getURL('assets/foo.html');
+};
+
+function testLoadAccessibleSubresourceInAppWebviewFrame() {
+  var webview = document.querySelector('webview');
+
+  webview.addEventListener('loadstop', function() {
+    var script = `
+      fetch('accessible.txt')
+        .then(response => response.text())
+        .then(data => {
+          if (data == 'Hello World\\n')
+            chrome.test.sendMessage('TEST_PASSED');
+          else 
+            throw new Error("Unexpected data: " + data);
+        })
+        .catch((error) => {
+          console.warn(error);
+          chrome.test.sendMessage('TEST_FAILED');
+        });
+    `;
+    webview.executeScript({code: script});
+  });
+
+  webview.src = chrome.runtime.getURL('assets/foo.html');
 };
 
 function testReloadWebviewAccessibleResource() {
@@ -120,7 +167,6 @@ function testLoadWebviewInaccessibleResource() {
     webview.executeScript({code: 'location="' + inaccessibleURL + '";'});
     didNavigate = true;
   });
-
   // The inaccessible URL should be blocked, and the webview should stay at
   // foo.html.
   webview.addEventListener('loadabort', function(e) {

chrome/test/data/extensions/platform_apps/web_view/load_webview_accessible_resource/manifest.json

@@ -14,7 +14,11 @@
     "partitions": [
       {
         "name": "foo",
-        "accessible_resources": ["assets/*"]
+        "accessible_resources": [
+          "assets/foo.html",
+          "assets/test.bmp",
+          "assets/accessible.txt"
+        ]
       }
     ]
   },

extensions/browser/url_request_util.cc

@@ -84,28 +84,21 @@ bool AllowCrossRendererResourceLoad(const GURL& url,
   if (resource_type == blink::mojom::ResourceType::kMainFrame) {
     *allowed = true;
     return true;
-  } else if (resource_type == blink::mojom::ResourceType::kSubFrame) {
-    // When navigating in subframe, allow if it is the same origin
-    // as the top-level frame. This can only be the case if the subframe
-    // request is coming from the extension process.
-    if (process_map.Contains(child_id)) {
-      *allowed = true;
-      return true;
-    }
+  }
 
-    // Also allow if the file is explicitly listed as a web_accessible_resource.
-    if (WebAccessibleResourcesInfo::IsResourceWebAccessible(
-            extension, resource_path.as_string())) {
-      *allowed = true;
-      return true;
-    }
+  // When navigating in subframe, allow if it is the same origin
+  // as the top-level frame. This can only be the case if the subframe
+  // request is coming from the extension process.
+  if (resource_type == blink::mojom::ResourceType::kSubFrame &&
+      process_map.Contains(child_id)) {
+    *allowed = true;
+    return true;
   }
 
-  // Since not all subresources are required to be listed in a v2
-  // manifest, we must allow all subresource loads if there are any web
-  // accessible resources. See http://crbug.com/179127.
-  if (!blink::IsResourceTypeFrame(resource_type) &&
-      WebAccessibleResourcesInfo::HasWebAccessibleResources(extension)) {
+  // Allow web accessible extension resources to be loaded as
+  // subresources/sub-frames.
+  if (WebAccessibleResourcesInfo::IsResourceWebAccessible(
+          extension, resource_path.as_string())) {
     *allowed = true;
     return true;
   }