[Backport] Security bug 914511

Manual backport of patch by Lukasz Anforowicz <lukasza@chromium.org>:
Ignore port numbers when matching isolated origins.

If IsolatedOrigins includes https://corp.example.com (defaulting to port
443), then https://corp.example.com:8443 should be isolated from
https://mail.example.com (even though 8443 is different from the port
number used when specifying which origins to isolate).

Bug: 914511
Reviewed-on: https://chromium-review.googlesource.com/c/1374524

Omitting changes to unit tests.

Change-Id: I892f81f79a2ee86812f4e367cbb6870676f0c3a2
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

3 files changed, 19 insertions, 10 deletions

diff --git a/chromium/content/browser/child_process_security_policy_impl.cc b/chromium/content/browser/child_process_security_policy_impl.cc
index 0f49f25f5f5..2b897e3508e 100644
--- a/chromium/content/browser/child_process_security_policy_impl.cc
+++ b/chromium/content/browser/child_process_security_policy_impl.cc
@@ -1246,6 +1246,19 @@ void ChildProcessSecurityPolicyImpl::AddIsolatedOrigins(
     return true;  // Remove.
   });
 
+  // Ports are ignored when matching isolated origins (see also
+  // https://crbug.com/914511).
+  for (url::Origin& origin : origins_to_add) {
+    const std::string& scheme = origin.scheme();
+    int default_port =
+        url::DefaultPortForScheme(scheme.data(), scheme.length());
+    if (origin.port() != default_port) {
+      LOG(ERROR) << "Ignoring port number in isolated origin: " << origin;
+      origin = url::Origin::Create(GURL(
+          origin.scheme() + url::kStandardSchemeSeparator + origin.host()));
+    }
+  }
+
   // Taking the lock once and doing a batch insertion via base::flat_set::insert
   // is important because of performance characteristics of base::flat_set.
   base::AutoLock lock(lock_);
diff --git a/chromium/content/browser/child_process_security_policy_impl.h b/chromium/content/browser/child_process_security_policy_impl.h
index 85a1b1d1277..bc47ed4d61f 100644
--- a/chromium/content/browser/child_process_security_policy_impl.h
+++ b/chromium/content/browser/child_process_security_policy_impl.h
@@ -231,10 +231,10 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
   // Returns true if sending system exclusive messages is allowed.
   bool CanSendMidiSysExMessage(int child_id);
 
-  // Add |origins| to the list of origins that require process isolation.
-  // When making process model decisions for such origins, the full
-  // scheme+host+port tuple rather than scheme and eTLD+1 will be used.
-  // SiteInstances for these origins will also use the full origin as site URL.
+  // Add |origins| to the list of origins that require process isolation.  When
+  // making process model decisions for such origins, the scheme+host tuple
+  // rather than scheme and eTLD+1 will be used.  SiteInstances for these
+  // origins will also use the full host of the isolated origin as site URL.
   //
   // Subdomains of an isolated origin are considered to be part of that
   // origin's site.  For example, if https://isolated.foo.com is added as an
@@ -262,9 +262,6 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
   // origin, this will return true for https://isolated.foo.com/,
   // https://bar.isolated.foo.com/, or https://baz.bar.isolated.foo.com/; and
   // it will return false for https://foo.com/ or https://unisolated.foo.com/.
-  //
-  // Note that unlike site URLs for regular web sites, isolated origins care
-  // about port.
   bool IsIsolatedOrigin(const url::Origin& origin);
 
   // Removes a previously added isolated origin, currently only used in tests.
diff --git a/chromium/content/browser/isolated_origin_util.cc b/chromium/content/browser/isolated_origin_util.cc
index fe25348a652..f5275c5ee92 100644
--- a/chromium/content/browser/isolated_origin_util.cc
+++ b/chromium/content/browser/isolated_origin_util.cc
@@ -18,12 +18,11 @@ bool IsolatedOriginUtil::DoesOriginMatchIsolatedOrigin(
   if (isolated_origin.GetURL().HostIsIPAddress())
     return origin == isolated_origin;
 
+  // Compare scheme and hostname, but don't compare ports - see
+  // https://crbug.com/914511.
   if (origin.scheme() != isolated_origin.scheme())
     return false;
 
-  if (origin.port() != isolated_origin.port())
-    return false;
-
   // Subdomains of an isolated origin are considered to be in the same isolated
   // origin.
   return origin.DomainIs(isolated_origin.host());