Merge r221978 - Make history.pushState()/replaceState() more closely …

…aligned to the HTML standard https://bugs.webkit.org/show_bug.cgi?id=176730 <rdar://problem/33839265> Reviewed by Alex Christensen. Source/WebCore: Update history.pushState()/replaceState() to more closely align with the algorithm specified in <https://html.spec.whatwg.org/multipage/history.html#dom-history-pushstate-2> (9 September 2017). Test: http/tests/security/history-pushState-replaceState-from-sandboxed-iframe.html * page/History.cpp: (WebCore::History::stateObjectAdded): * page/SecurityOrigin.cpp: (WebCore::SecurityOrigin::extractInnerURL): Use URL constructor that takes a base URL as opposed to using the special ParsedURLString-variant because the latter can only be used to parse a string returned from URL::string(). And the extracted inner URL does not meet this criterion. Using the ParsedURLString-variant of the URL constructor with a string that is not the result of URL::string() will cause an assertion failure in a debug build. LayoutTests: * http/tests/security/history-pushState-replaceState-from-sandboxed-iframe-expected.txt: Added. * http/tests/security/history-pushState-replaceState-from-sandboxed-iframe.html: Added. * http/tests/security/history-username-password-expected.txt: * http/tests/security/history-username-password.html: * http/tests/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html: Added.
WebKit · Nov 8, 2017 · 0450a2f · 0450a2f
1 parent ab9d166
commit 0450a2f
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 19 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2017-09-13  Daniel Bates  <dabates@apple.com>
+
+        Make history.pushState()/replaceState() more closely aligned to the HTML standard
+        https://bugs.webkit.org/show_bug.cgi?id=176730
+        <rdar://problem/33839265>
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/security/history-pushState-replaceState-from-sandboxed-iframe-expected.txt: Added.
+        * http/tests/security/history-pushState-replaceState-from-sandboxed-iframe.html: Added.
+        * http/tests/security/history-username-password-expected.txt:
+        * http/tests/security/history-username-password.html:
+        * http/tests/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html: Added.
+
 2017-10-20  Joanmarie Diggs  <jdiggs@igalia.com>
 
         AX: [ATK] Events missing and state incorrect for aria-activedescendant

diff --git a/...sts/http/tests/security/history-pushState-replaceState-from-sandboxed-iframe-expected.txt b/...sts/http/tests/security/history-pushState-replaceState-from-sandboxed-iframe-expected.txt
@@ -0,0 +1,22 @@
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+Tests history.replaceState(), history.pushState() from a sandboxed iframe
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.history.replaceState(null, "New title", location.href) did not throw exception.
+PASS window.history.pushState(null, "New title", location.href) did not throw exception.
+PASS window.history.replaceState(null, "New title", completeURL("")) threw exception SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/. Paths and fragments must match for a sandboxed document..
+PASS window.history.pushState(null, "New title", completeURL("")) threw exception SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/. Paths and fragments must match for a sandboxed document..
+PASS window.history.replaceState(null, "New title", completeURL("dummy")) threw exception SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/dummy. Paths and fragments must match for a sandboxed document..
+PASS window.history.pushState(null, "New title", completeURL("dummy")) threw exception SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/dummy. Paths and fragments must match for a sandboxed document..
+PASS window.history.replaceState(null, "New title", completeURL("", "dummy")) threw exception SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/?dummy. Paths and fragments must match for a sandboxed document..
+PASS window.history.pushState(null, "New title", completeURL("", "dummy")) threw exception SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/?dummy. Paths and fragments must match for a sandboxed document..
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/security/history-pushState-replaceState-from-sandboxed-iframe.html b/LayoutTests/http/tests/security/history-pushState-replaceState-from-sandboxed-iframe.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.dumpChildFramesAsText();
+}
+</script>
+</head>
+<body>
+<iframe src="resources/history-pushState-replaceState-from-sandboxed-iframe.html" sandbox="allow-scripts" width="100%" height="700"></iframe>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/history-username-password-expected.txt b/LayoutTests/http/tests/security/history-username-password-expected.txt
@@ -1,14 +1,18 @@
 Click to test in new window
-SecurityError: Attempt to use history.replaceState() to change session history URL to http://www.webkit.org@127.0.0.1:8000/ is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.replaceState() to change session history URL to http://:www.webkit.org@127.0.0.1:8000/ is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.replaceState() to change session history URL to http://www.webkit:org@127.0.0.1:8000/ is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.pushState() to add URL http://www.webkit.org@127.0.0.1:8000/ to session history is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.pushState() to add URL http://:www.webkit.org@127.0.0.1:8000/ to session history is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.pushState() to add URL http://www.webkit:org@127.0.0.1:8000/ to session history is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.replaceState() to change session history URL to http://www.webkit.org@127.0.0.1:8000/ is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.replaceState() to change session history URL to http://:www.webkit.org@127.0.0.1:8000/ is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.replaceState() to change session history URL to http://www.webkit:org@127.0.0.1:8000/ is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.pushState() to add URL http://www.webkit.org@127.0.0.1:8000/ to session history is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.pushState() to add URL http://:www.webkit.org@127.0.0.1:8000/ to session history is insecure; Username/passwords aren't allowed in state object URLs
-SecurityError: Attempt to use history.pushState() to add URL http://www.webkit:org@127.0.0.1:8000/ to session history is insecure; Username/passwords aren't allowed in state object URLs
+SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/history-username-password.html to http://www.webkit.org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/history-username-password.html to http://:www.webkit.org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/history-username-password.html to http://www.webkit:org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/history-username-password.html to blob:http://www.webkit:org@127.0.0.1:8000. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/history-username-password.html to http://www.webkit.org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/history-username-password.html to http://:www.webkit.org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/history-username-password.html to http://www.webkit:org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/history-username-password.html to blob:http://www.webkit:org@127.0.0.1:8000. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.replaceState() to change session history URL from about:blank to http://www.webkit.org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.replaceState() to change session history URL from about:blank to http://:www.webkit.org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.replaceState() to change session history URL from about:blank to http://www.webkit:org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.replaceState() to change session history URL from about:blank to blob:http://www.webkit:org@127.0.0.1:8000. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.pushState() to change session history URL from about:blank to http://www.webkit.org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.pushState() to change session history URL from about:blank to http://:www.webkit.org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.pushState() to change session history URL from about:blank to http://www.webkit:org@127.0.0.1:8000/. Protocols, domains, ports, usernames, and passwords must match.
+SecurityError: Blocked attempt to use history.pushState() to change session history URL from about:blank to blob:http://www.webkit:org@127.0.0.1:8000. Protocols, domains, ports, usernames, and passwords must match.
 
diff --git a/LayoutTests/http/tests/security/history-username-password.html b/LayoutTests/http/tests/security/history-username-password.html
@@ -33,6 +33,13 @@
         log(e);
     }
 
+    try {
+        historyToTest.replaceState(null, "Phishy Title", "blob:" + location.protocol + "//www.webkit:org" + "@" + location.host);
+        log("replaceState with username and password worked, shouldn't have.");
+    } catch(e) {
+        log(e);
+    }
+
     try {
         historyToTest.pushState(null, "Phishy Title", location.protocol + "//www.webkit.org" + "@" + location.host);
         log("pushState with username worked, shouldn't have.");
@@ -53,6 +60,13 @@
     } catch(e) {
         log(e);
     }
+
+    try {
+        historyToTest.pushState(null, "Phishy Title", "blob:" + location.protocol + "//www.webkit:org" + "@" + location.host);
+        log("pushState with username and password worked, shouldn't have.");
+    } catch(e) {
+        log(e);
+    }
 }
 
 function clicked()

diff --git a/...s/http/tests/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html b/...s/http/tests/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<script src="/js-test-resources/js-test.js"></script>
+<body>
+<script>
+function completeURL(path = "", fragment = "")
+{
+    var url = `${location.protocol}//${location.host}/`;
+    if (path)
+        url += path;
+    if (fragment)
+        url += "?" + fragment;
+    return url;
+}
+
+description("Tests history.replaceState(), history.pushState() from a sandboxed iframe");
+
+shouldNotThrow('window.history.replaceState(null, "New title", location.href)');
+shouldNotThrow('window.history.pushState(null, "New title", location.href)');
+
+shouldThrow('window.history.replaceState(null, "New title", completeURL(""))', "'SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/. Paths and fragments must match for a sandboxed document.'");
+
+shouldThrow('window.history.pushState(null, "New title", completeURL(""))', "'SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/. Paths and fragments must match for a sandboxed document.'");
+
+shouldThrow('window.history.replaceState(null, "New title", completeURL("dummy"))', "'SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/dummy. Paths and fragments must match for a sandboxed document.'");
+
+shouldThrow('window.history.pushState(null, "New title", completeURL("dummy"))', "'SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/dummy. Paths and fragments must match for a sandboxed document.'");
+
+shouldThrow('window.history.replaceState(null, "New title", completeURL("", "dummy"))', "'SecurityError: Blocked attempt to use history.replaceState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/?dummy. Paths and fragments must match for a sandboxed document.'");
+
+shouldThrow('window.history.pushState(null, "New title", completeURL("", "dummy"))', "'SecurityError: Blocked attempt to use history.pushState() to change session history URL from http://127.0.0.1:8000/security/resources/history-pushState-replaceState-from-sandboxed-iframe.html to http://127.0.0.1:8000/?dummy. Paths and fragments must match for a sandboxed document.'");
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2017-09-13  Daniel Bates  <dabates@apple.com>
+
+        Make history.pushState()/replaceState() more closely aligned to the HTML standard
+        https://bugs.webkit.org/show_bug.cgi?id=176730
+        <rdar://problem/33839265>
+
+        Reviewed by Alex Christensen.
+
+        Update history.pushState()/replaceState() to more closely align with the algorithm
+        specified in <https://html.spec.whatwg.org/multipage/history.html#dom-history-pushstate-2> (9 September 2017).
+
+        Test: http/tests/security/history-pushState-replaceState-from-sandboxed-iframe.html
+
+        * page/History.cpp:
+        (WebCore::History::stateObjectAdded):
+        * page/SecurityOrigin.cpp:
+        (WebCore::SecurityOrigin::extractInnerURL): Use URL constructor that takes a base URL as opposed
+        to using the special ParsedURLString-variant because the latter can only be used to parse a string
+        returned from URL::string(). And the extracted inner URL does not meet this criterion. Using the
+        ParsedURLString-variant of the URL constructor with a string that is not the result of URL::string()
+        will cause an assertion failure in a debug build.
+
 2017-10-25  Ryosuke Niwa  <rniwa@webkit.org>
 
         Style::Scope::flushPendingUpdate() can replace the entire document in XSLTProcessor::createDocumentFromSource

diff --git a/Source/WebCore/page/History.cpp b/Source/WebCore/page/History.cpp
@@ -174,14 +174,19 @@ ExceptionOr<void> History::stateObjectAdded(RefPtr<SerializedScriptValue>&& data
         return { };
 
     URL fullURL = urlForState(urlString);
-    if (!fullURL.isValid() || !m_frame->document()->securityOrigin().canRequest(fullURL))
+    if (!fullURL.isValid())
         return Exception { SecurityError };
 
-    if (fullURL.hasUsername() || fullURL.hasPassword()) {
-        if (stateObjectType == StateObjectType::Replace)
-            return Exception { SecurityError, "Attempt to use history.replaceState() to change session history URL to " + fullURL.string() + " is insecure; Username/passwords aren't allowed in state object URLs" };
-        return Exception { SecurityError, "Attempt to use history.pushState() to add URL " + fullURL.string() + " to session history is insecure; Username/passwords aren't allowed in state object URLs" };
-    }
+    const URL& documentURL = m_frame->document()->url();
+
+    auto createBlockedURLSecurityErrorWithMessageSuffix = [&] (const char* suffix) {
+        const char* functionName = stateObjectType == StateObjectType::Replace ? "history.replaceState()" : "history.pushState()";
+        return Exception { SecurityError, makeString("Blocked attempt to use ", functionName, " to change session history URL from ", documentURL.stringCenterEllipsizedToLength(), " to ", fullURL.stringCenterEllipsizedToLength(), ". ", suffix) };
+    };
+    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()))
+        return createBlockedURLSecurityErrorWithMessageSuffix("Paths and fragments must match for a sandboxed document.");
 
     Document* mainDocument = m_frame->page()->mainFrame().document();
     History* mainHistory = nullptr;

diff --git a/Source/WebCore/page/SecurityOrigin.cpp b/Source/WebCore/page/SecurityOrigin.cpp
@@ -66,7 +66,7 @@ URL SecurityOrigin::extractInnerURL(const URL& url)
 {
     // FIXME: Update this callsite to use the innerURL member function when
     // we finish implementing it.
-    return URL(ParsedURLString, decodeURLEscapeSequences(url.path()));
+    return { URL(), decodeURLEscapeSequences(url.path()) };
 }
 
 static RefPtr<SecurityOrigin> getCachedOrigin(const URL& url)