Add a query and fragment exception to history API's unique origin restriction.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Mar 2018 17:53:39 +0000 (17:53 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Mar 2018 17:53:39 +0000 (17:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183028

Patch by Danyao Wang <danyao@chromium.org> on 2018-03-12
Reviewed by Brent Fulgham.

Tests: http/tests/navigation/pushstate-at-unique-origin-denied.php
       Tools/TestWebKitAPI/Tests/WebCore/URL.cpp

* page/History.cpp:
(WebCore::History::stateObjectAdded):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229540 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/http/tests/navigation/pushstate-at-unique-origin-denied-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/navigation/pushstate-at-unique-origin-denied.php [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/History.cpp
Source/WebCore/platform/URL.cpp
Source/WebCore/platform/URL.h
Tools/TestWebKitAPI/Tests/WebCore/URL.cpp

diff --git a/LayoutTests/http/tests/navigation/pushstate-at-unique-origin-denied-expected.txt b/LayoutTests/http/tests/navigation/pushstate-at-unique-origin-denied-expected.txt
new file mode 100644 (file)
index 0000000..dc3e907
--- /dev/null
@@ -0,0 +1,6 @@
+
+PASS pushState to a new path in unique origin should fail with SecurityError 
+PASS pushState #hash in unique origin should not fail with SecurityError 
+PASS pushState ?query in unique origin should not fail with SecurityError 
+PASS pushState ?query#hash in unique origin should not fail with SecurityError 
+
diff --git a/LayoutTests/http/tests/navigation/pushstate-at-unique-origin-denied.php b/LayoutTests/http/tests/navigation/pushstate-at-unique-origin-denied.php
new file mode 100644 (file)
index 0000000..d75b768
--- /dev/null
@@ -0,0 +1,40 @@
+<?php
+header("Content-Security-Policy: sandbox allow-scripts");
+?>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<script>
+var originURL = document.URL;
+test(function () {
+    assert_throws('SecurityError', function () {
+        history.pushState(null, null, originURL + "/path");
+    });
+}, 'pushState to a new path in unique origin should fail with SecurityError');
+
+test(function () {
+    try {
+        history.pushState(null, null, originURL + "#hash");
+        done();
+    } catch (e) {
+        assert_unreached("pushState #hash should not fail.");
+    }
+}, 'pushState #hash in unique origin should not fail with SecurityError');
+
+test(function () {
+    try {
+        history.pushState(null, null, originURL + "?query");
+        done();
+    } catch (e) {
+        assert_unreached("pushState ?query should not fail.");
+    }
+}, 'pushState ?query in unique origin should not fail with SecurityError');
+
+test(function () {
+    try {
+        history.pushState(null, null, originURL + "?query#hash");
+        done();
+    } catch (e) {
+        assert_unreached("pushState ?query#hash should not fail.");
+    }
+}, 'pushState ?query#hash in unique origin should not fail with SecurityError');
+</script>
index 9169b47..4c9c6b9 100644 (file)
@@ -1,3 +1,16 @@
+2018-03-12  Danyao Wang  <danyao@chromium.org>
+
+        Add a query and fragment exception to history API's unique origin restriction.
+        https://bugs.webkit.org/show_bug.cgi?id=183028
+
+        Reviewed by Brent Fulgham.
+
+        Tests: http/tests/navigation/pushstate-at-unique-origin-denied.php
+               Tools/TestWebKitAPI/Tests/WebCore/URL.cpp
+
+        * page/History.cpp:
+        (WebCore::History::stateObjectAdded):
+
 2018-03-12  Antti Koivisto  <antti@apple.com>
 
         Don't invalidate descendants for nth pseudo classes unless needed
index ad48313..ae2047c 100644 (file)
@@ -192,7 +192,13 @@ ExceptionOr<void> History::stateObjectAdded(RefPtr<SerializedScriptValue>&& data
     };
     if (!protocolHostAndPortAreEqual(fullURL, documentURL) || fullURL.user() != documentURL.user() || fullURL.pass() != documentURL.pass())
         return createBlockedURLSecurityErrorWithMessageSuffix("Protocols, domains, ports, usernames, and passwords must match.");
-    if (!m_frame->document()->securityOrigin().canRequest(fullURL) && (fullURL.path() != documentURL.path() || fullURL.query() != documentURL.query()))
+
+    const auto& documentSecurityOrigin = m_frame->document()->securityOrigin();
+    // We allow sandboxed documents, 'data:'/'file:' URLs, etc. to use 'pushState'/'replaceState' to modify the URL query and fragments.
+    // See https://bugs.webkit.org/show_bug.cgi?id=183028 for the compatibility concerns.
+    bool allowSandboxException = (documentSecurityOrigin.isLocal() || documentSecurityOrigin.isUnique()) && equalIgnoringQueryAndFragment(documentURL, fullURL);
+
+    if (!allowSandboxException && !documentSecurityOrigin.canRequest(fullURL) && (fullURL.path() != documentURL.path() || fullURL.query() != documentURL.query()))
         return createBlockedURLSecurityErrorWithMessageSuffix("Paths and fragments must match for a sandboxed document.");
 
     Document* mainDocument = m_frame->page()->mainFrame().document();
index 97d91f3..c2629de 100644 (file)
@@ -729,6 +729,18 @@ bool equalIgnoringFragmentIdentifier(const URL& a, const URL& b)
     return true;
 }
 
+bool equalIgnoringQueryAndFragment(const URL& a, const URL& b)
+{
+    if (a.pathEnd() != b.pathEnd())
+        return false;
+    unsigned pathEnd = a.pathEnd();
+    for (unsigned i = 0; i < pathEnd; ++i) {
+        if (a.string()[i] != b.string()[i])
+            return false;
+    }
+    return true;
+}
+
 bool protocolHostAndPortAreEqual(const URL& a, const URL& b)
 {
     if (a.m_schemeEnd != b.m_schemeEnd)
index 45a7b31..1c05d99 100644 (file)
@@ -309,6 +309,7 @@ bool operator!=(const URL&, const String&);
 bool operator!=(const String&, const URL&);
 
 WEBCORE_EXPORT bool equalIgnoringFragmentIdentifier(const URL&, const URL&);
+WEBCORE_EXPORT bool equalIgnoringQueryAndFragment(const URL&, const URL&);
 WEBCORE_EXPORT bool protocolHostAndPortAreEqual(const URL&, const URL&);
 WEBCORE_EXPORT bool hostsAreEqual(const URL&, const URL&);
 
index e07b475..dc8c002 100644 (file)
@@ -213,6 +213,108 @@ TEST_F(URLTest, URLRemoveQueryAndFragmentIdentifier)
     EXPECT_EQ(url.string(), url5.string());
 }
 
+TEST_F(URLTest, EqualIgnoringFragmentIdentifier)
+{
+    struct TestCase {
+        const char* url1;
+        const char* url2;
+        bool expected;
+    } cases[] = {
+        {"http://example.com/", "http://example.com/", true},
+        {"http://example.com/#hash", "http://example.com/", true},
+        {"http://example.com/path", "http://example.com/", false},
+        {"http://example.com/path", "http://example.com/path", true},
+        {"http://example.com/path#hash", "http://example.com/path", true},
+        {"http://example.com/path?query", "http://example.com/path", false},
+        {"http://example.com/path?query#hash", "http://example.com/path", false},
+        {"http://example.com/otherpath", "http://example.com/path", false},
+        {"http://example.com:80/", "http://example.com/", true},
+        {"http://example.com:80/#hash", "http://example.com/", true},
+        {"http://example.com:80/path", "http://example.com/", false},
+        {"http://example.com:80/path#hash", "http://example.com/path", true},
+        {"http://example.com:80/path?query", "http://example.com/path", false},
+        {"http://example.com:80/path?query#hash", "http://example.com/path", false},
+        {"http://example.com:80/otherpath", "http://example.com/path", false},
+        {"http://not-example.com:80/", "http://example.com/", false},
+        {"http://example.com:81/", "http://example.com/", false},
+        {"http://example.com:81/#hash", "http://example.com:81/", true},
+        {"http://example.com:81/path", "http://example.com:81", false},
+        {"http://example.com:81/path#hash", "http://example.com:81/path", true},
+        {"http://example.com:81/path?query", "http://example.com:81/path", false},
+        {"http://example.com:81/path?query#hash", "http://example.com:81/path", false},
+        {"http://example.com:81/otherpath", "http://example.com:81/path", false},
+        {"file:///path/to/file.html", "file:///path/to/file.html", true},
+        {"file:///path/to/file.html#hash", "file:///path/to/file.html", true},
+        {"file:///path/to/file.html?query", "file:///path/to/file.html", false},
+        {"file:///path/to/file.html?query#hash", "file:///path/to/file.html", false},
+        {"file:///path/to/other_file.html", "file:///path/to/file.html", false},
+        {"file:///path/to/other/file.html", "file:///path/to/file.html", false},
+        {"data:text/plain;charset=utf-8;base64,76O/76O/76O/", "data:text/plain;charset=utf-8;base64,760/760/760/", false},
+        {"http://example.com", "file://example.com", false},
+        {"http://example.com/#hash", "file://example.com", false},
+        {"http://example.com/?query", "file://example.com/", false},
+        {"http://example.com/?query#hash", "file://example.com/", false},
+    };
+
+    for (const auto& test : cases) {
+        URL url1 = createURL(test.url1);
+        URL url2 = createURL(test.url2);
+        EXPECT_EQ(test.expected, equalIgnoringFragmentIdentifier(url1, url2))
+            << "Test failed for " << test.url1 << " vs. " << test.url2;
+    }
+}
+
+TEST_F(URLTest, EqualIgnoringQueryAndFragment)
+{
+    struct TestCase {
+        const char* url1;
+        const char* url2;
+        bool expected;
+    } cases[] = {
+        {"http://example.com/", "http://example.com/", true},
+        {"http://example.com/#hash", "http://example.com/", true},
+        {"http://example.com/path", "http://example.com/", false},
+        {"http://example.com/path", "http://example.com/path", true},
+        {"http://example.com/path#hash", "http://example.com/path", true},
+        {"http://example.com/path?query", "http://example.com/path", true},
+        {"http://example.com/path?query#hash", "http://example.com/path", true},
+        {"http://example.com/otherpath", "http://example.com/path", false},
+        {"http://example.com:80/", "http://example.com/", true},
+        {"http://example.com:80/#hash", "http://example.com/", true},
+        {"http://example.com:80/path", "http://example.com/", false},
+        {"http://example.com:80/path#hash", "http://example.com/path", true},
+        {"http://example.com:80/path?query", "http://example.com/path", true},
+        {"http://example.com:80/path?query#hash", "http://example.com/path", true},
+        {"http://example.com:80/otherpath", "http://example.com/path", false},
+        {"http://not-example.com:80/", "http://example.com:80/", false},
+        {"http://example.com:81/", "http://example.com/", false},
+        {"http://example.com:81/#hash", "http://example.com:81/", true},
+        {"http://example.com:81/path", "http://example.com:81", false},
+        {"http://example.com:81/path#hash", "http://example.com:81/path", true},
+        {"http://example.com:81/path?query", "http://example.com:81/path", true},
+        {"http://example.com:81/path?query#hash", "http://example.com:81/path", true},
+        {"http://example.com:81/otherpath", "http://example.com:81/path", false},
+        {"file:///path/to/file.html", "file:///path/to/file.html", true},
+        {"file:///path/to/file.html#hash", "file:///path/to/file.html", true},
+        {"file:///path/to/file.html?query", "file:///path/to/file.html", true},
+        {"file:///path/to/file.html?query#hash", "file:///path/to/file.html", true},
+        {"file:///path/to/other_file.html", "file:///path/to/file.html", false},
+        {"file:///path/to/other/file.html", "file:///path/to/file.html", false},
+        {"data:text/plain;charset=utf-8;base64,76O/76O/76O/", "data:text/plain;charset=utf-8;base64,760/760/760/", false},
+        {"http://example.com", "file://example.com", false},
+        {"http://example.com/#hash", "file://example.com", false},
+        {"http://example.com/?query", "file://example.com/", false},
+        {"http://example.com/?query#hash", "file://example.com/", false},
+    };
+
+    for (const auto& test : cases) {
+        URL url1 = createURL(test.url1);
+        URL url2 = createURL(test.url2);
+        EXPECT_EQ(test.expected, equalIgnoringQueryAndFragment(url1, url2))
+            << "Test failed for " << test.url1 << " vs. " << test.url2;
+    }
+}
+
 TEST_F(URLTest, ProtocolIsInHTTPFamily)
 {
     EXPECT_FALSE(protocolIsInHTTPFamily({}));