Skip to content

Commit

Permalink
Validate item URL in BackForwardAddItem() IPC
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=244620
<rdar://98999288>

Reviewed by Brent Fulgham.

When the client approves a navigation to a file URL, we keep track of it.
We then use this information to validate the URL of the item in the
BackForwardAddItem() IPC coming from the WebProcess.

Also, update history.pushState() / replaceState() to throw if the new URL
is a file URL and its path does not match the path of the current URL.
This aligns WebKit's behavior with Blink and the specification:
- https://html.spec.whatwg.org/multipage/history.html#can-have-its-url-rewritten (Step 4)

This Web-exposed change is important since trying to call history.pushState()
or replaceState() with a different file path would trip the IPC check I
am adding in this patch.

* LayoutTests/fast/loader/stateobjects/pushstate-frequency-iframe.html:
* LayoutTests/fast/loader/stateobjects/pushstate-frequency.html:
* LayoutTests/fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt:
* LayoutTests/fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange.html:
* LayoutTests/fast/loader/stateobjects/replacestate-frequency-iframe.html:
* LayoutTests/fast/loader/stateobjects/replacestate-frequency.html:
* LayoutTests/fast/loader/stateobjects/resources/pushstate-iframe.html:
* LayoutTests/fast/loader/stateobjects/resources/replacestate-iframe.html:
* LayoutTests/fast/loader/stateobjects/state-url-sets-links-visited.html:
Update existing tests to reflect the Web-exposed changes to history.pushState() / replaceState().

* Source/WebCore/page/History.cpp:
(WebCore::History::stateObjectAdded):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationActionAsyncShared):
(WebKit::WebPageProxy::backForwardAddItem):
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::addPreviouslyApprovedFileURL):
(WebKit::WebProcessProxy::wasPreviouslyApprovedFileURL const):
* Source/WebKit/UIProcess/WebProcessProxy.h:

Canonical link: https://commits.webkit.org/256785@main
  • Loading branch information
cdumez committed Nov 17, 2022
1 parent 30b1c33 commit 78a55cf
Show file tree
Hide file tree
Showing 34 changed files with 169 additions and 83 deletions.

This file was deleted.

Expand Up @@ -14,7 +14,7 @@
{
try {
for( var i = 0; i < 75; ++i ) {
history.pushState(0, 0, i.toString());
history.pushState(0, 0, "#" + i);
log("Successfully added item: " + i);
}
} catch (e) {
Expand Down
Expand Up @@ -12,7 +12,7 @@
window.onload = function() {
try {
for( var i = 0; i < 100000; ++i ) {
history.pushState(0, 0, i.toString());
history.pushState(0, 0, "#" + i);
log("Successfully added item: " + i);
}
} catch (e) {
Expand Down
@@ -1,26 +1,30 @@
This test pushes a series of state objects with different URLs and fragment identifiers meant to test the hashChange event as states are popped.

State popped with event null (type object) and last path component some-other.html?withsomeotherquery
State popped with event null (type object) and last path component some-other.html?withsomeotherquery#
Hash change fired and last path component is some-other.html?withsomeotherquery#
State popped with event null (type object) and last path component some-other.html?withsomeotherquery
Hash change fired and last path component is some-other.html?withsomeotherquery
State popped with event null (type object) and last path component some-other.html?withsomeotherquery#somehash
Hash change fired and last path component is some-other.html?withsomeotherquery#somehash
State popped with event null (type object) and last path component some-other.html?withsomeotherquery#someotherhash
Hash change fired and last path component is some-other.html?withsomeotherquery#someotherhash
State popped with event null (type object) and last path component some-other.html?withquery#someotherhash
State popped with event null (type object) and last path component some-other.html?withquery#
Hash change fired and last path component is some-other.html?withquery#
State popped with event null (type object) and last path component some-other.html?withquery#somehash
Hash change fired and last path component is some-other.html?withquery#somehash
State popped with event null (type object) and last path component some-other.html?withquery
Hash change fired and last path component is some-other.html?withquery
State popped with event null (type object) and last path component some-other.html?withquery#
Hash change fired and last path component is some-other.html?withquery#
State popped with event null (type object) and last path component some-other.html?withquery
Hash change fired and last path component is some-other.html?withquery
State popped with event null (type object) and last path component some-other.html
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS history.pushState(null, null, 'some-other.html') threw
PASS history.replaceState(null, null, 'some-other.html') threw
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery#
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery#
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery#somehash
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery#somehash
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery#someotherhash
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withsomeotherquery#someotherhash
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withquery#someotherhash
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withquery#
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withquery#
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withquery#somehash
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withquery#somehash
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withquery
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withquery
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withquery#
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withquery#
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html?withquery
Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html?withquery
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#
State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#otherhash
Expand Down
@@ -1,39 +1,43 @@
<html>
<head>
<script src="../../../resources/js-test.js"></script>
<script>

if (window.testRunner) {
if (window.testRunner)
testRunner.clearBackForwardList();
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

function log(txt)
{
document.getElementById("logger").innerText += txt + "\n";
}

function runTest()
{
try {
history.pushState(null, null, 'some-other.html');
testFailed("history.pushState(null, null, 'some-other.html') didn't throw");
} catch (e) {
testPassed("history.pushState(null, null, 'some-other.html') threw");
}
try {
history.replaceState(null, null, 'some-other.html');
testFailed("history.replaceState(null, null, 'some-other.html') didn't throw");
} catch (e) {
testPassed("history.replaceState(null, null, 'some-other.html') threw");
}
history.replaceState("OriginalEntry", "original");
history.pushState(null, null, "#");
history.pushState(null, null, "");
history.pushState(null, null, "#hash");
history.pushState(null, null, "#otherhash");
history.pushState(null, null, "#");
history.pushState(null, null, null);
history.pushState(null, null, "some-other.html");
history.pushState(null, null, "some-other.html?withquery");
history.pushState(null, null, "some-other.html?withquery#");
history.pushState(null, null, "some-other.html?withquery");
history.pushState(null, null, "some-other.html?withquery#somehash");
history.pushState(null, null, "some-other.html?withquery#");
history.pushState(null, null, "some-other.html?withquery#someotherhash");
history.pushState(null, null, "some-other.html?withsomeotherquery#someotherhash");
history.pushState(null, null, "some-other.html?withsomeotherquery#somehash");
history.pushState(null, null, "some-other.html?withsomeotherquery");
history.pushState(null, null, "some-other.html?withsomeotherquery#");
history.pushState(null, null, "some-other.html?withsomeotherquery");
history.pushState(null, null, "?withquery");
history.pushState(null, null, "?withquery#");
history.pushState(null, null, "?withquery");
history.pushState(null, null, "?withquery#somehash");
history.pushState(null, null, "?withquery#");
history.pushState(null, null, "?withquery#someotherhash");
history.pushState(null, null, "?withsomeotherquery#someotherhash");
history.pushState(null, null, "?withsomeotherquery#somehash");
history.pushState(null, null, "?withsomeotherquery");
history.pushState(null, null, "?withsomeotherquery#");
history.pushState(null, null, "?withsomeotherquery");

history.pushState("BufferEntry", "Last entry");
history.back();
Expand All @@ -46,7 +50,7 @@

onpopstate = function(event)
{
log("State popped with event " + event.state + " (type " + typeof event.state + ") and last path component " + lastPathComponent(location.href));
debug("State popped with event " + event.state + " (type " + typeof event.state + ") and last path component " + lastPathComponent(location.href));
if (event.state != "OriginalEntry")
setTimeout("history.back();", 0);
else if (window.testRunner)
Expand All @@ -55,14 +59,14 @@

onhashchange = function(event)
{
log("Hash change fired and last path component is " + lastPathComponent(event.newURL));
debug("Hash change fired and last path component is " + lastPathComponent(event.newURL));
}

</script>
<body onload="runTest();">
<pre>
This test pushes a series of state objects with different URLs and fragment identifiers meant to test the hashChange event as states are popped.
</pre><br>
<pre id="logger"></pre>
<script>
description("This test pushes a series of state objects with different URLs and fragment identifiers meant to test the hashChange event as states are popped.");
jsTestIsAsync = true;
</script>
</body>
</html>
Expand Up @@ -14,7 +14,7 @@
{
try {
for( var i = 0; i < 75; ++i ) {
history.replaceState(0, 0, i.toString());
history.replaceState(0, 0, "#" + i);
log("Successfully added item: " + i);
}
} catch (e) {
Expand Down
Expand Up @@ -12,7 +12,7 @@
window.onload = function() {
try {
for( var i = 0; i < 100000; ++i ) {
history.replaceState(0, 0, i.toString());
history.replaceState(0, 0, "#" + i);
log("Successfully added item: " + i);
}
} catch (e) {
Expand Down
Expand Up @@ -14,7 +14,7 @@
log("Adding state objects in iframe");
try {
for( var i = 0; i < 75; ++i ) {
history.pushState(0, 0, i.toString());
history.pushState(0, 0, "#" + i);
log("Successfully added item: " + i);
}
} catch (e) {
Expand Down
Expand Up @@ -14,7 +14,7 @@
log("Adding state objects in iframe");
try {
for( var i = 0; i < 75; ++i ) {
history.replaceState(0, 0, i.toString());
history.replaceState(0, 0, "#" + i);
log("Successfully added item: " + i);
}
} catch (e) {
Expand Down
Expand Up @@ -11,8 +11,8 @@
{
description('Verify that changes done by history.replaceState and history.pushState update visitedLinks.');

window.history.replaceState(null, "Title", "replacedURL.html");
window.history.pushState(null, "Title", "pushedURL.html");
window.history.replaceState(null, "Title", "#foo");
window.history.pushState(null, "Title", "#bar");

if (window.internals) {
style1 = internals.computedStyleIncludingVisitedInfo(document.getElementById('link1'));
Expand All @@ -34,8 +34,8 @@
</head>
<body onload="loaded();">
<p id=description></p>
<a id="link1" href="replacedURL.html">This link should get colored visited as a result of replaceState() setting it as the current URL</a><br>
<a id="link2" href="pushedURL.html">This link should get colored visited as a result of pushState() adding it to the forward list</a><br>
<a id="link1" href="#foo">This link should get colored visited as a result of replaceState() setting it as the current URL</a><br>
<a id="link2" href="#bar">This link should get colored visited as a result of pushState() adding it to the forward list</a><br>
If you're running in a browser, the link should be orange-on-black and you should see "replacedURL.html" in your global history.<br>
If you're running in DRT, the test will also append "PASS" or "FAIL".<br>
<div id=console></div>
Expand Down
@@ -0,0 +1,7 @@
main frame - has 1 onunload handler(s)
PASS

============== Back Forward List ==============
curr-> http://127.0.0.1:8000/history/resources/history-replace-updates-current-item-done.html **nav target**
http://127.0.0.1:8000/history/resources/history-replace-updates-current-item-goback.html **nav target**
===============================================
2 changes: 1 addition & 1 deletion LayoutTests/loader/stateobjects/pushstate-size-iframe.html
Expand Up @@ -32,7 +32,7 @@
function clicked()
{
try {
history.pushState(object, object, object);
history.pushState(object, object, "#" + object);
} catch (e) {
log("Unexpected exception: " + e);
if (window.testRunner)
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/loader/stateobjects/pushstate-size.html
Expand Up @@ -28,7 +28,7 @@
function clicked()
{
try {
history.pushState(object, object, object);
history.pushState(object, object, "#" + object);
} catch (e) {
log("User gesture: " + e);
if (window.testRunner)
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/loader/stateobjects/replacestate-size-iframe.html
Expand Up @@ -22,15 +22,15 @@
try {
// Push 59+mb of state objects (while the string is exactly 1mb, it counts for a little more than 1mb)
for (var i = 1; i < 60; ++i)
history.pushState(0, 0, object);
history.pushState(0, 0, "#" + object);
} catch (e) {
log("Unexpected exception pushing > 59mb but < 60mb of state objects: " + e);
}

log("Replace the last 1mb object with a 2mb object, bringing the total to 60+mb");
object += object;
try {
history.replaceState(0, 0, object);
history.replaceState(0, 0, "#" + object);
log("It fit.");
} catch (e) {
log("Unexpected exception replacing 1mb with 2mb, bringing the total to 60mb: " + e);
Expand Down
6 changes: 3 additions & 3 deletions LayoutTests/loader/stateobjects/replacestate-size.html
Expand Up @@ -21,7 +21,7 @@
try {
// Push 63+mb of state objects (while the string is exactly 1mb, it counts for a little more than 1mb)
for (var i = 1; i < 64; ++i)
history.pushState(0, 0, object);
history.pushState(0, 0, "#" + object);
} catch (e) {
log("Unexpected exception pushing > 63mb but < 64mb of state objects: " + e);
}
Expand All @@ -32,7 +32,7 @@
// replaceState with a 1mb state object.
// If replaceState counts against the global limit, this will cause an exception.
// But it should work.
history.replaceState(0, 0, object);
history.replaceState(0, 0, "#" + object);

log("It fit");
} catch (e) {
Expand All @@ -46,7 +46,7 @@

try {
// Replacing the 63rd 1mb string with a 2mb string *should* still trigger the limit.
history.replaceState(0, 0, object);
history.replaceState(0, 0, "#" + object);
log("It fit, but should not have");
} catch (e) {
log("Expected exception replacing the last state object: " + e);
Expand Down
Expand Up @@ -18,7 +18,7 @@
function iframeClicked()
{
try {
history.pushState(object, object, object);
history.pushState(object, object, "#" + object);
} catch (e) {
log("Expected exception: " + e);
if (window.testRunner)
Expand Down
Expand Up @@ -19,7 +19,7 @@
log("The total payload is currently 60+mb. Pushing a 1mb object brings that 61+mb.");

try {
history.pushState(0, 0, object);
history.pushState(0, 0, "#" + object);
log("It fit.");
} catch (e) {
log("Unexpected exception pushing 1mb:" + e);
Expand All @@ -29,7 +29,7 @@

object += object;
try {
history.replaceState(0, 0, object);
history.replaceState(0, 0, "#" + object);
log("It fit.");
} catch (e) {
log("Unexpected exception replacing 1mb with 2mb:" + e);
Expand All @@ -39,7 +39,7 @@

object += object;
try {
history.replaceState(0, 0, object);
history.replaceState(0, 0, "#" + object);
log("It fit, but should not have.");
} catch (e) {
log("Expected exception replacing 2mb with 4mb:" + e);
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/platform/wk2/TestExpectations
Expand Up @@ -457,7 +457,7 @@ plugins/destroy-on-setwindow.html

# New history tests added in r76205 fail on WebKit2 (perhaps the change
# as coded doesn't affect both processes for WebKit2)
fast/history/history-replace-updates-current-item.html
http/tests/history/history-replace-updates-current-item.html
http/tests/navigation/redirect-on-back-updates-history-item.html
http/tests/navigation/redirect-on-reload-updates-history-item.html

Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/swipe/pushState-cached-back-swipe.html
Expand Up @@ -75,7 +75,7 @@

await UIHelper.delayFor(0);

history.pushState(null, null, "/second");
history.pushState(null, null, "#second");
updateContent();

await UIHelper.delayFor(0);
Expand Down
Expand Up @@ -111,7 +111,7 @@

await UIHelper.delayFor(0);

history.pushState(null, null, "/second");
history.pushState(null, null, "#second");
updateContent();

await UIHelper.delayFor(0);
Expand Down
1 change: 1 addition & 0 deletions Source/WTF/wtf/cocoa/RuntimeApplicationChecksCocoa.h
Expand Up @@ -79,6 +79,7 @@ enum class SDKAlignedBehavior {
ObservesClassProperty,
PictureInPictureMediaPlayback,
ProcessSwapOnCrossSiteNavigation,
PushStateFilePathRestriction,
RequiresUserGestureToLoadVideo,
RestrictsBaseURLSchemes,
ScrollViewContentInsetsAreNotObscuringInsets,
Expand Down
12 changes: 12 additions & 0 deletions Source/WebCore/page/History.cpp
Expand Up @@ -43,6 +43,10 @@
#include <wtf/MainThread.h>
#include <wtf/text/StringConcatenateNumbers.h>

#if PLATFORM(COCOA)
#include <wtf/cocoa/RuntimeApplicationChecksCocoa.h>
#endif

namespace WebCore {

WTF_MAKE_ISO_ALLOCATED_IMPL(History);
Expand Down Expand Up @@ -204,6 +208,14 @@ ExceptionOr<void> History::stateObjectAdded(RefPtr<SerializedScriptValue>&& data
if (!protocolHostAndPortAreEqual(fullURL, documentURL) || fullURL.user() != documentURL.user() || fullURL.password() != documentURL.password())
return createBlockedURLSecurityErrorWithMessageSuffix("Protocols, domains, ports, usernames, and passwords must match.");

if (fullURL.isLocalFile()
#if PLATFORM(COCOA)
&& linkedOnOrAfterSDKWithBehavior(SDKAlignedBehavior::PushStateFilePathRestriction)
#endif
&& fullURL.fileSystemPath() != documentURL.fileSystemPath()) {
return createBlockedURLSecurityErrorWithMessageSuffix("Only differences in query and fragment are allowed for file: URLs.");
}

const auto& documentSecurityOrigin = 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.
Expand Down

0 comments on commit 78a55cf

Please sign in to comment.