Always deny a permissions request for different virtual vs loaded URLs

Virtual URLs are used in almost all UI display contexts in Chrome, but
do not always represent the page that was actually loaded in the
renderer. If the scheme is HTTP or HTTPS, and the virtual and loaded
URLs are totally different origins, automatically deny the request.

This should basically never happen because all but one virtual URL
handlers are for chrome:// pages. The only http handler is for a
special type of Preview which will have JavaScript disabled and should
not create permission dialogs. None the less, if one does get created,
it should be denied so that the user doesn't approve a request for the
wrong page.

Bug: 881938
Change-Id: Iedb835f72e0a963347ed2a85dc2a71dc43e1f53c
Reviewed-on: https://chromium-review.googlesource.com/c/1260082
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597302}

chrome/browser/permissions/permission_context_base.cc

@@ -32,6 +32,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
@@ -167,6 +168,7 @@ void PermissionContextBase::RequestPermission(
         break;
       case PermissionStatusSource::INSECURE_ORIGIN:
       case PermissionStatusSource::UNSPECIFIED:
+      case PermissionStatusSource::VIRTUAL_URL_DIFFERENT_ORIGIN:
         break;
     }
 
@@ -230,6 +232,31 @@ PermissionResult PermissionContextBase::GetPermissionStatus(
                             PermissionStatusSource::FEATURE_POLICY);
   }
 
+  if (render_frame_host) {
+    content::WebContents* web_contents =
+        content::WebContents::FromRenderFrameHost(render_frame_host);
+
+    // Automatically deny all HTTP or HTTPS requests where the virtual URL and
+    // the loaded URL are for different origins. The loaded URL is the one
+    // actually in the renderer, but the virtual URL is the one
+    // seen by the user. This may be very confusing for a user to see in a
+    // permissions request.
+    const content::NavigationEntry* entry =
+        web_contents->GetController().GetLastCommittedEntry();
+    if (entry) {
+      const GURL virtual_url = entry->GetVirtualURL();
+      const GURL loaded_url = entry->GetURL();
+      if (virtual_url.SchemeIsHTTPOrHTTPS() &&
+          loaded_url.SchemeIsHTTPOrHTTPS() &&
+          !url::Origin::Create(virtual_url)
+               .IsSameOriginWith(url::Origin::Create(loaded_url))) {
+        return PermissionResult(
+            CONTENT_SETTING_BLOCK,
+            PermissionStatusSource::VIRTUAL_URL_DIFFERENT_ORIGIN);
+      }
+    }
+  }
+
   ContentSetting content_setting = GetPermissionStatusInternal(
       render_frame_host, requesting_origin, embedding_origin);
   if (content_setting == CONTENT_SETTING_ASK) {

chrome/browser/permissions/permission_context_base_unittest.cc

@@ -36,6 +36,7 @@
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/mock_render_process_host.h"
@@ -594,6 +595,23 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
     EXPECT_EQ(response, permission_context.GetContentSettingFromMap(url, url));
   }
 
+  void TestVirtualURL(const GURL& loaded_url,
+                      const GURL& virtual_url,
+                      const ContentSetting want_response,
+                      const PermissionStatusSource& want_source) {
+    TestPermissionContext permission_context(
+        profile(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
+
+    NavigateAndCommit(loaded_url);
+    web_contents()->GetController().GetVisibleEntry()->SetVirtualURL(
+        virtual_url);
+
+    PermissionResult result = permission_context.GetPermissionStatus(
+        web_contents()->GetMainFrame(), virtual_url, virtual_url);
+    EXPECT_EQ(result.content_setting, want_response);
+    EXPECT_EQ(result.source, want_source);
+  }
+
   void SetUpUrl(const GURL& url) {
     NavigateAndCommit(url);
     prompt_factory_->DocumentOnLoadCompletedInMainFrame();
@@ -707,3 +725,20 @@ TEST_F(PermissionContextBaseTests, TestParallelRequestsBlocked) {
 TEST_F(PermissionContextBaseTests, TestParallelRequestsDismissed) {
   TestParallelRequests(CONTENT_SETTING_ASK);
 }
+
+TEST_F(PermissionContextBaseTests, TestVirtualURLDifferentOrigin) {
+  TestVirtualURL(GURL("http://www.google.com"), GURL("http://foo.com"),
+                 CONTENT_SETTING_BLOCK,
+                 PermissionStatusSource::VIRTUAL_URL_DIFFERENT_ORIGIN);
+}
+
+TEST_F(PermissionContextBaseTests, TestVirtualURLNotHTTP) {
+  TestVirtualURL(GURL("chrome://foo"), GURL("chrome://newtab"),
+                 CONTENT_SETTING_ASK, PermissionStatusSource::UNSPECIFIED);
+}
+
+TEST_F(PermissionContextBaseTests, TestVirtualURLSameOrigin) {
+  TestVirtualURL(GURL("http://www.google.com"),
+                 GURL("http://www.google.com/foo"), CONTENT_SETTING_ASK,
+                 PermissionStatusSource::UNSPECIFIED);
+}

chrome/browser/permissions/permission_result.h

@@ -29,6 +29,12 @@ enum class PermissionStatusSource {
 
   // The feature has been blocked in the requesting frame by feature policy.
   FEATURE_POLICY,
+
+  // The virtual URL and the loaded URL are for different origins. The loaded
+  // URL is the one actually in the renderer, but the virtual URL is the one
+  // seen by the user. This may be very confusing for a user to see in a
+  // permissions request.
+  VIRTUAL_URL_DIFFERENT_ORIGIN,
 };
 
 struct PermissionResult {

chrome/browser/permissions/permission_uma_util.cc

@@ -206,6 +206,7 @@ void PermissionUmaUtil::RecordEmbargoPromptSuppressionFromSource(
     case PermissionStatusSource::KILL_SWITCH:
     case PermissionStatusSource::INSECURE_ORIGIN:
     case PermissionStatusSource::FEATURE_POLICY:
+    case PermissionStatusSource::VIRTUAL_URL_DIFFERENT_ORIGIN:
       // The permission wasn't under embargo, so don't record anything. We may
       // embargo it later.
       break;