[Backport] Security issue 973628

Avoid rewriting about:srcdoc into chrome://srcdoc

Rewriting about:srcdoc into chrome://srcdoc is undesirable because

1. about:srcdoc has a special meaning and just like about:blank has been
    reserved by specs like
    https://html.spec.whatwg.org/multipage/urls-and-fetching.html

2. chrome:-scheme URLs are special and might have extra privileges.
    Therefore chrome: URLs should not be reachable by an unprivileged webpage
    (OTOH, the rewriting fixed here only applies to the URL *shown* to
    the user, not the URL that gets committed - compare WebContents's
    GetVisibleURL vs GetLastCommittedURL).

Bug: 973628
Change-Id: I021e623caf0d7e5c02a2546291bb4913412b3125
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669328}
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

7 files changed, 72 insertions, 17 deletions

diff --git a/chromium/components/url_formatter/url_fixer.cc b/chromium/components/url_formatter/url_fixer.cc
index e1f05c59264..941e31da87c 100644
--- a/chromium/components/url_formatter/url_fixer.cc
+++ b/chromium/components/url_formatter/url_fixer.cc
@@ -551,11 +551,11 @@ GURL FixupURL(const std::string& text, const std::string& desired_tld) {
     return GURL();
   }
 
-  // 'about:blank' is special-cased in various places in the code so it
-  // shouldn't be transformed into 'chrome://blank' as the code below will do.
+  // 'about:blank' and 'about:srcdoc' are special-cased in various places in the
+  // code and shouldn't use the chrome: scheme.
   if (base::LowerCaseEqualsASCII(scheme, url::kAboutScheme)) {
     GURL about_url(base::ToLowerASCII(trimmed));
-    if (about_url.IsAboutBlank())
+    if (about_url.IsAboutBlank() || about_url.IsAboutSrcdoc())
       return about_url;
   }
 
diff --git a/chromium/components/url_formatter/url_fixer_unittest.cc b/chromium/components/url_formatter/url_fixer_unittest.cc
index 939e1916c58..97014ecef53 100644
--- a/chromium/components/url_formatter/url_fixer_unittest.cc
+++ b/chromium/components/url_formatter/url_fixer_unittest.cc
@@ -279,9 +279,9 @@ static bool IsMatchingFileURL(const std::string& url,
   if (url.length() <= 8)
     return false;
   if (std::string("file:///") != url.substr(0, 8))
-    return false; // no file:/// prefix
+    return false;  // no file:/// prefix
   if (url.find('\\') != std::string::npos)
-    return false; // contains backslashes
+    return false;  // contains backslashes
 
   base::FilePath derived_path;
   net::FileURLToFilePath(GURL(url), &derived_path);
@@ -307,6 +307,11 @@ struct FixupCase {
     {"about:version", "chrome://version/"},
     {"about:blank", "about:blank"},
     {"About:blaNk", "about:blank"},
+    {"about:blank#blah", "about:blank#blah"},
+    {"about:blank/#blah", "about:blank/#blah"},
+    {"about:srcdoc", "about:srcdoc"},
+    {"about:srcdoc#blah", "about:srcdoc#blah"},
+    {"about:srcdoc/#blah", "about:srcdoc/#blah"},
     {"about:usr:pwd@hst:20/pth?qry#ref", "chrome://hst/pth?qry#ref"},
     {"about://usr:pwd@hst/pth?qry#ref", "chrome://hst/pth?qry#ref"},
     {"chrome:usr:pwd@hst/pth?qry#ref", "chrome://hst/pth?qry#ref"},
diff --git a/chromium/url/gurl.cc b/chromium/url/gurl.cc
index c77706b7ad9..41f564bfcca 100644
--- a/chromium/url/gurl.cc
+++ b/chromium/url/gurl.cc
@@ -8,6 +8,7 @@
 
 #include <algorithm>
 #include <ostream>
+#include <utility>
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
@@ -344,16 +345,11 @@ bool GURL::IsCustom() const {
 }
 
 bool GURL::IsAboutBlank() const {
-  if (!SchemeIs(url::kAboutScheme))
-    return false;
-
-  if (has_host() || has_username() || has_password() || has_port())
-    return false;
-
-  if (path() != url::kAboutBlankPath && path() != url::kAboutBlankWithHashPath)
-    return false;
+  return IsAboutUrl(url::kAboutBlankPath);
+}
 
-  return true;
+bool GURL::IsAboutSrcdoc() const {
+  return IsAboutUrl(url::kAboutSrcdocPath);
 }
 
 bool GURL::SchemeIs(base::StringPiece lower_ascii_scheme) const {
@@ -478,6 +474,30 @@ size_t GURL::EstimateMemoryUsage() const {
          (parsed_.inner_parsed() ? sizeof(url::Parsed) : 0);
 }
 
+bool GURL::IsAboutUrl(base::StringPiece allowed_path) const {
+  if (!SchemeIs(url::kAboutScheme))
+    return false;
+
+  if (has_host() || has_username() || has_password() || has_port())
+    return false;
+
+  if (!path_piece().starts_with(allowed_path))
+    return false;
+
+  if (path_piece().size() == allowed_path.size()) {
+    DCHECK_EQ(path_piece(), allowed_path);
+    return true;
+  }
+
+  if ((path_piece().size() == allowed_path.size() + 1) &&
+      path_piece().back() == '/') {
+    DCHECK_EQ(path_piece(), allowed_path.as_string() + '/');
+    return true;
+  }
+
+  return false;
+}
+
 std::ostream& operator<<(std::ostream& out, const GURL& url) {
   return out << url.possibly_invalid_spec();
 }
diff --git a/chromium/url/gurl.h b/chromium/url/gurl.h
index 4f7f2fe8b6b..c978df56ceb 100644
--- a/chromium/url/gurl.h
+++ b/chromium/url/gurl.h
@@ -220,6 +220,10 @@ class COMPONENT_EXPORT(URL) GURL {
   // about:blank/#foo.
   bool IsAboutBlank() const;
 
+  // Returns true when the url is of the form about:srcdoc, about:srcdoc?foo or
+  // about:srcdoc/#foo.
+  bool IsAboutSrcdoc() const;
+
   // Returns true if the given parameter (should be lower-case ASCII to match
   // the canonicalized scheme) is the scheme for this URL. Do not include a
   // colon.
@@ -447,6 +451,9 @@ class COMPONENT_EXPORT(URL) GURL {
 
   void InitializeFromCanonicalSpec();
 
+  // Helper used by IsAboutBlank and IsAboutSrcdoc.
+  bool IsAboutUrl(base::StringPiece allowed_path) const;
+
   // Returns the substring of the input identified by the given component.
   std::string ComponentString(const url::Component& comp) const {
     if (comp.len <= 0)
diff --git a/chromium/url/gurl_unittest.cc b/chromium/url/gurl_unittest.cc
index caf5dd4e1b3..e22ed714d44 100644
--- a/chromium/url/gurl_unittest.cc
+++ b/chromium/url/gurl_unittest.cc
@@ -855,11 +855,34 @@ TEST(GURLTest, IsAboutBlank) {
   const std::string kNotAboutBlankUrls[] = {
       "http:blank",      "about:blan",          "about://blank",
       "about:blank/foo", "about://:8000/blank", "about://foo:foo@/blank",
-      "foo@about:blank", "foo:bar@about:blank", "about:blank:8000"};
+      "foo@about:blank", "foo:bar@about:blank", "about:blank:8000",
+      "about:blANk"};
   for (const auto& url : kNotAboutBlankUrls)
     EXPECT_FALSE(GURL(url).IsAboutBlank()) << url;
 }
 
+TEST(GURLTest, IsAboutSrcdoc) {
+  const std::string kAboutSrcdocUrls[] = {
+      "about:srcdoc", "about:srcdoc/", "about:srcdoc?foo", "about:srcdoc/#foo",
+      "about:srcdoc?foo#foo"};
+  for (const auto& url : kAboutSrcdocUrls)
+    EXPECT_TRUE(GURL(url).IsAboutSrcdoc()) << url;
+
+  const std::string kNotAboutSrcdocUrls[] = {"http:srcdoc",
+                                             "about:srcdo",
+                                             "about://srcdoc",
+                                             "about://srcdoc\\",
+                                             "about:srcdoc/foo",
+                                             "about://:8000/srcdoc",
+                                             "about://foo:foo@/srcdoc",
+                                             "foo@about:srcdoc",
+                                             "foo:bar@about:srcdoc",
+                                             "about:srcdoc:8000",
+                                             "about:srCDOc"};
+  for (const auto& url : kNotAboutSrcdocUrls)
+    EXPECT_FALSE(GURL(url).IsAboutSrcdoc()) << url;
+}
+
 TEST(GURLTest, EqualsIgnoringRef) {
   const struct {
     const char* url_a;
diff --git a/chromium/url/url_constants.cc b/chromium/url/url_constants.cc
index 110c6a7b22d..38f86bbbf23 100644
--- a/chromium/url/url_constants.cc
+++ b/chromium/url/url_constants.cc
@@ -9,7 +9,7 @@ namespace url {
 const char kAboutBlankURL[] = "about:blank";
 
 const char kAboutBlankPath[] = "blank";
-const char kAboutBlankWithHashPath[] = "blank/";
+const char kAboutSrcdocPath[] = "srcdoc";
 
 const char kAboutScheme[] = "about";
 const char kBlobScheme[] = "blob";
diff --git a/chromium/url/url_constants.h b/chromium/url/url_constants.h
index 38a0e38ce8a..7f322f89787 100644
--- a/chromium/url/url_constants.h
+++ b/chromium/url/url_constants.h
@@ -14,7 +14,7 @@ namespace url {
 COMPONENT_EXPORT(URL) extern const char kAboutBlankURL[];
 
 COMPONENT_EXPORT(URL) extern const char kAboutBlankPath[];
-COMPONENT_EXPORT(URL) extern const char kAboutBlankWithHashPath[];
+COMPONENT_EXPORT(URL) extern const char kAboutSrcdocPath[];
 
 COMPONENT_EXPORT(URL) extern const char kAboutScheme[];
 COMPONENT_EXPORT(URL) extern const char kBlobScheme[];