Skip to content

Commit

Permalink
Calling evaluateJavaScript enables back-button hijacking
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261611
rdar://115561250

Reviewed by Ben Nham.

In 253405@main, I updated our back/forward list hijacking prevention logic by
treating history items added by JS (e.g. via 'history.pushState()`) as having
a user gesture if a user gesture had occurred in the last 10 seconds. This was
needed for backward compatibility with some legit sites.

The issue now is that if the client app has called evaluateJavaScript on the
WKWebView in the last 10 seconds, the JS will be able to hijack the back/forward
list again.

In 265168@main, we did some hardening so that the transient activation gets
consumed after the evaluateJavaScript call has completed. However, it didn't
fix the back/forward list hijacking prevention logic because it relies on
user gesture and not transient activation.

To address the issue, I updated out back/forward list hijacking prevention logic
to rely on transient user activation rather than whether or not there was a
user gesture in the last 10 minutes.

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::hasRecentUserInteractionForNavigationFromJS const):
* Tools/TestWebKitAPI/Tests/WebKit/WKBackForwardListTests.mm:
(TEST):

Canonical link: https://commits.webkit.org/272448.685@safari-7618-branch
  • Loading branch information
cdumez committed Mar 6, 2024
1 parent cd62341 commit 028628c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
8 changes: 6 additions & 2 deletions Source/WebCore/dom/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8180,8 +8180,12 @@ bool Document::hasRecentUserInteractionForNavigationFromJS() const
if (UserGestureIndicator::processingUserGesture(this))
return true;

static constexpr Seconds maximumItervalForUserGestureForwarding { 10_s };
return (MonotonicTime::now() - lastHandledUserGestureTimestamp()) <= maximumItervalForUserGestureForwarding;
RefPtr window = domWindow();
if (!window || window->lastActivationTimestamp().isInfinity())
return false;

static constexpr Seconds maximumIntervalForUserActivationForwarding { 10_s };
return (MonotonicTime::now() - window->lastActivationTimestamp()) <= maximumIntervalForUserActivationForwarding;
}

void Document::startTrackingStyleRecalcs()
Expand Down
14 changes: 14 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKit/WKBackForwardListTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,20 @@ static void runBackForwardNavigationSkipsItemsWithoutUserGestureTest(Function<vo
});
}

TEST(WKBackForwardList, BackForwardNavigationSkipsItemsWithoutUserGesturePushStateAfterEvaluateJS)
{
runBackForwardNavigationSkipsItemsWithoutUserGestureTest([](WKWebView* webView, ASCIILiteral destination) {
// Do a call to evaluateJavaScript (with user gesture) *BEFORE* the pushState and make sure it doesn't count
// as a user gesture for the pushState().
__block bool didRunScript = false;
[webView evaluateJavaScript:@"window.foo = 1;" completionHandler:^(id, NSError *) {
didRunScript = true;
}];
TestWebKitAPI::Util::run(&didRunScript);
[webView _evaluateJavaScriptWithoutUserGesture:makeString("history.pushState(null, document.title, "_s, destination, ");"_s) completionHandler:nil];
});
}

TEST(WKBackForwardList, BackForwardNavigationSkipsItemsWithoutUserGestureSubframe)
{
TestWebKitAPI::HTTPServer server({
Expand Down

0 comments on commit 028628c

Please sign in to comment.