Skip to content

Commit

Permalink
[macOS] Pointer Lock should disengage when client windows present a s…
Browse files Browse the repository at this point in the history
…heet

https://bugs.webkit.org/show_bug.cgi?id=268198
rdar://121694233

Reviewed by Aditya Keerthi.

The Pointer Lock API is susceptible to abuse by nefarious webpages since
they can (programmatically or otherwise) make client windows show alerts
or permission granting sheets while pointer lock is engaged. Since our
current implementation of pointer lock stays engaged even when the
client window presents a sheet, it leaves the user in a compromised
state where they both don't know the location of the mouse cursor and
don't have a way to exit the pointer lock state (since the client window
where pointer lock is engaged is no longer focused or the key window).

This patch addresses this vulnerability by registering observers for the
NSWindowWillBeginSheetNotification notification on the WebView's current
window, and then requesting for pointer lock to be disengaged whenever
we receive a notification that said window will begin presenting a
sheet.

Test case added in WebKit.ClientDisplaysAlertSheetWhilePointerLockActive
that asserts we successfully exit pointer lock when a client window
presents an alert sheet. It also tests that we can successfully re-enter
pointer lock afterwards.

* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(-[WKWindowVisibilityObserver startObserving:]):
(-[WKWindowVisibilityObserver stopObserving:]):
(-[WKWindowVisibilityObserver _windowWillBeginSheet:]):
(WebKit::WebViewImpl::windowWillBeginSheet):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm:
(-[PointerLockDelegate resetState]):
(-[PointerLockDelegate waitForPointerLockEngaged]):
(-[PointerLockDelegate waitForPointerLockLost]):
(-[PointerLockDelegate _webViewDidRequestPointerLock:completionHandler:]):
(-[PointerLockDelegate _webViewDidLosePointerLock:]):

Originally-landed-as: 272448.388@safari-7618-branch (7047e8e). rdar://124554592
Canonical link: https://commits.webkit.org/276163@main
  • Loading branch information
aprotyas authored and robert-jenner committed Mar 15, 2024
1 parent b160146 commit 4ad05d7
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/WebPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -1655,6 +1655,7 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ
#if ENABLE(POINTER_LOCK)
void didAllowPointerLock();
void didDenyPointerLock();
void requestPointerUnlock();
#endif

void setSuppressVisibilityUpdates(bool flag);
Expand Down Expand Up @@ -2451,7 +2452,6 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ

#if ENABLE(POINTER_LOCK)
void requestPointerLock();
void requestPointerUnlock();
#endif

void didCreateMainFrame(WebCore::FrameIdentifier);
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/mac/WebViewImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ class WebViewImpl : public CanMakeWeakPtr<WebViewImpl>, public CanMakeCheckedPtr
void windowDidDeminiaturize();
void windowDidMove();
void windowDidResize();
void windowWillBeginSheet();
void windowDidChangeBackingProperties(CGFloat oldBackingScaleFactor);
void windowDidChangeScreen();
void windowDidChangeLayerHosting();
Expand Down
14 changes: 14 additions & 0 deletions Source/WebKit/UIProcess/mac/WebViewImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ - (void)startObserving:(NSWindow *)window
[defaultNotificationCenter addObserver:self selector:@selector(_windowDidDeminiaturize:) name:NSWindowDidDeminiaturizeNotification object:window];
[defaultNotificationCenter addObserver:self selector:@selector(_windowDidMove:) name:NSWindowDidMoveNotification object:window];
[defaultNotificationCenter addObserver:self selector:@selector(_windowDidResize:) name:NSWindowDidResizeNotification object:window];
[defaultNotificationCenter addObserver:self selector:@selector(_windowWillBeginSheet:) name:NSWindowWillBeginSheetNotification object:window];
[defaultNotificationCenter addObserver:self selector:@selector(_windowDidChangeBackingProperties:) name:NSWindowDidChangeBackingPropertiesNotification object:window];
[defaultNotificationCenter addObserver:self selector:@selector(_windowDidChangeScreen:) name:NSWindowDidChangeScreenNotification object:window];
[defaultNotificationCenter addObserver:self selector:@selector(_windowDidChangeLayerHosting:) name:_NSWindowDidChangeContentsHostedInLayerSurfaceNotification object:window];
Expand Down Expand Up @@ -361,6 +362,7 @@ - (void)stopObserving:(NSWindow *)window
[defaultNotificationCenter removeObserver:self name:NSWindowDidDeminiaturizeNotification object:window];
[defaultNotificationCenter removeObserver:self name:NSWindowDidMoveNotification object:window];
[defaultNotificationCenter removeObserver:self name:NSWindowDidResizeNotification object:window];
[defaultNotificationCenter removeObserver:self name:NSWindowWillBeginSheetNotification object:window];
[defaultNotificationCenter removeObserver:self name:NSWindowDidChangeBackingPropertiesNotification object:window];
[defaultNotificationCenter removeObserver:self name:NSWindowDidChangeScreenNotification object:window];
[defaultNotificationCenter removeObserver:self name:_NSWindowDidChangeContentsHostedInLayerSurfaceNotification object:window];
Expand Down Expand Up @@ -437,6 +439,11 @@ - (void)_windowDidResize:(NSNotification *)notification
_impl->windowDidResize();
}

- (void)_windowWillBeginSheet:(NSNotification *)notification
{
_impl->windowWillBeginSheet();
}

- (void)_windowDidChangeBackingProperties:(NSNotification *)notification
{
CGFloat oldBackingScaleFactor = [[notification.userInfo objectForKey:NSBackingPropertyOldScaleFactorKey] doubleValue];
Expand Down Expand Up @@ -2038,6 +2045,13 @@ static bool isInRecoveryOS()
updateWindowAndViewFrames();
}

void WebViewImpl::windowWillBeginSheet()
{
#if ENABLE(POINTER_LOCK)
m_page->requestPointerUnlock();
#endif
}

void WebViewImpl::windowDidChangeBackingProperties(CGFloat oldBackingScaleFactor)
{
CGFloat newBackingScaleFactor = intrinsicDeviceScaleFactor();
Expand Down
60 changes: 57 additions & 3 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -683,14 +683,41 @@ - (void)webView:(WKWebView *)webView stopURLSchemeTask:(id <WKURLSchemeTask>)url
}

@interface PointerLockDelegate : NSObject <WKUIDelegatePrivate>
- (void)resetState;
- (void)waitForPointerLockEngaged;
- (void)waitForPointerLockLost;
@end

@implementation PointerLockDelegate
@implementation PointerLockDelegate {
bool _didEngagePointerLock;
bool _didLosePointerLock;
}

- (void)resetState
{
_didEngagePointerLock = false;
_didLosePointerLock = false;
}

- (void)waitForPointerLockEngaged
{
TestWebKitAPI::Util::run(&_didEngagePointerLock);
}

- (void)waitForPointerLockLost
{
TestWebKitAPI::Util::run(&_didLosePointerLock);
}

- (void)_webViewDidRequestPointerLock:(WKWebView *)webView completionHandler:(void (^)(BOOL))completionHandler
{
completionHandler(YES);
done = true;
_didEngagePointerLock = true;
}

- (void)_webViewDidLosePointerLock:(WKWebView *)webView
{
_didLosePointerLock = true;
}

@end
Expand All @@ -707,7 +734,34 @@ - (void)_webViewDidRequestPointerLock:(WKWebView *)webView completionHandler:(vo
@"</script>"
];
[webView sendClicksAtPoint:NSMakePoint(200, 200) numberOfClicks:1];
TestWebKitAPI::Util::run(&done);
[delegate waitForPointerLockEngaged];
}

TEST(WebKit, ClientDisplaysAlertSheetWhilePointerLockActive)
{
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600)]);
auto delegate = adoptNS([[PointerLockDelegate alloc] init]);
[webView setUIDelegate:delegate.get()];
[webView synchronouslyLoadHTMLString:
@"<canvas width='800' height='600'></canvas><script>"
@"var canvas = document.querySelector('canvas');"
@"canvas.onclick = ()=>{canvas.requestPointerLock()};"
@"</script>"
];
[webView sendClicksAtPoint:NSMakePoint(200, 200) numberOfClicks:1];
[delegate waitForPointerLockEngaged];
[delegate resetState];

// Check that pointer lock is lost upon sheet presentation.
auto alert = adoptNS([[NSAlert alloc] init]);
[alert beginSheetModalForWindow:[webView hostWindow] completionHandler:^(NSModalResponse) { }];
[delegate waitForPointerLockLost];
[[webView hostWindow] endSheet:[alert window]];
[delegate resetState];

// Check that pointer lock can be requested again successfully.
[webView sendClicksAtPoint:NSMakePoint(200, 200) numberOfClicks:1];
[delegate waitForPointerLockEngaged];
}

static bool receivedWindowFrame;
Expand Down

0 comments on commit 4ad05d7

Please sign in to comment.