[webkit-changes] [WebKit/WebKit] b16014: Bad cast under CachedResourceLoader::preload()

Fri Mar 15 09:38:21 PDT 2024

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: b160146fcad134e97707386c01bbee11913822e1
      https://github.com/WebKit/WebKit/commit/b160146fcad134e97707386c01bbee11913822e1
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2024-03-15 (Fri, 15 Mar 2024)

  Changed paths:
    M Source/WebCore/loader/cache/CachedResourceLoader.cpp

  Log Message:
  -----------
  Bad cast under CachedResourceLoader::preload()
https://bugs.webkit.org/show_bug.cgi?id=268405
rdar://121745788

Reviewed by Brent Fulgham.

In CachedResourceLoader::preload() we were calling requestResource(type)
to get a resource. Then if the type we requested was `FontResource`, we
assumed the the CachedResource returned was a CachedFont and would cast
to that type. However, this cast ends up being incorrect in some cases.
I suspect this could happen when requesting resources with the same URL
but different types.

To address the issue, we now check the actual type of the returned
CachedResource before casting it.

* Source/WebCore/loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::preload):

Originally-landed-as: 272448.421 at safari-7618-branch (25c11453b693). rdar://124554524
Canonical link: https://commits.webkit.org/276162@main

  Commit: 4ad05d788033d176b41c250c29fd5a524df20851
      https://github.com/WebKit/WebKit/commit/4ad05d788033d176b41c250c29fd5a524df20851
  Author: Abrar Rahman Protyasha <a_protyasha at apple.com>
  Date:   2024-03-15 (Fri, 15 Mar 2024)

  Changed paths:
    M Source/WebKit/UIProcess/WebPageProxy.h
    M Source/WebKit/UIProcess/mac/WebViewImpl.h
    M Source/WebKit/UIProcess/mac/WebViewImpl.mm
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm

  Log Message:
  -----------
  [macOS] Pointer Lock should disengage when client windows present a sheet
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 at safari-7618-branch (7047e8e918da). rdar://124554592
Canonical link: https://commits.webkit.org/276163@main

  Commit: 5f24a7c2d2e021a4074f009be7dc1a4537aa7b84
      https://github.com/WebKit/WebKit/commit/5f24a7c2d2e021a4074f009be7dc1a4537aa7b84
  Author: Erica Li <lerica at apple.com>
  Date:   2024-03-15 (Fri, 15 Mar 2024)

  Changed paths:
    A LayoutTests/media/audio-remove-playback-crash-expected.txt
    A LayoutTests/media/audio-remove-playback-crash.html
    M Source/WebCore/dom/Document.cpp

  Log Message:
  -----------
  ASAN_ILL | WebCore::Document::removePlaybackTargetPickerClient.
rdar://120661908

Reviewed by Chris Dumez.

Unable to ref the page from removePlaybackTargetPickerClient as it may have started destruction.

* LayoutTests/media/audio-remove-playback-crash-expected.txt: Added.
* LayoutTests/media/audio-remove-playback-crash.html: Added.
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::removePlaybackTargetPickerClient):

Originally-landed-as: 272448.387 at safari-7618-branch (303478e273bd). rdar://124554687
Canonical link: https://commits.webkit.org/276164@main

  Commit: a0ed946e698fa554e70676482787431c07ede0c9
      https://github.com/WebKit/WebKit/commit/a0ed946e698fa554e70676482787431c07ede0c9
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2024-03-15 (Fri, 15 Mar 2024)

  Changed paths:
    M Source/WebCore/editing/TextManipulationController.cpp

  Log Message:
  -----------
  Bad cast in TextManipulationController::scheduleObservationUpdate()
https://bugs.webkit.org/show_bug.cgi?id=268235
rdar://121646850

Reviewed by Wenson Hsieh.

Convert the downcast<>() into a dynamicDowncast<>() since the common ancestor
is not guaranteed to be an Element.

I have not been able to reproduce but it is happening in the wild.

* Source/WebCore/editing/TextManipulationController.cpp:
(WebCore::TextManipulationController::scheduleObservationUpdate):

Originally-landed-as: 272448.385 at safari-7618-branch (2d3dc03ecbbc). rdar://124554893
Canonical link: https://commits.webkit.org/276165@main

  Commit: 1e46cd53f762f408365e6b5ca24e692186aa1217
      https://github.com/WebKit/WebKit/commit/1e46cd53f762f408365e6b5ca24e692186aa1217
  Author: Simon Fraser <simon.fraser at apple.com>
  Date:   2024-03-15 (Fri, 15 Mar 2024)

  Changed paths:
    M Source/WebCore/page/scrolling/mac/ScrollerMac.mm
    M Source/WebCore/page/scrolling/mac/ScrollerPairMac.h
    M Source/WebCore/page/scrolling/mac/ScrollerPairMac.mm

  Log Message:
  -----------
  heap-use-after-free in WebCore::ScrollingTreeScrollingNode::scrollbarVisibilityDidChange
https://bugs.webkit.org/show_bug.cgi?id=267788
<rdar://119677269>

Reviewed by Chris Dumez.

We register a `WebScrollerImpPairDelegateMac` with a `NSScrollerImpPair` (an AppKit class), so its lifetime
is somewhat unknown. The `WebScrollerImpPairDelegateMac` references a `ScrollerPairMac` by raw pointer, which
should be cleaned up by `-invalidate` but convert it to a `ThreadSafeWeakPtr<>` for safety.

The larger issue is that the `ScrollerPairMac` stored a `ScrollingTreeScrollingNode` by reference, so
nothing guaranteed that the `ScrollingTreeScrollingNode` outlived the `ScrollerPairMac`. Convert this to
a `ThreadSafeWeakPtr<>` also.

I was not able to reproduce the bug with the provided testcase.

* Source/WebCore/page/scrolling/mac/ScrollerMac.mm:
(WebCore::ScrollerMac::visibilityChanged):
(WebCore::ScrollerMac::updateMinimumKnobLength):
* Source/WebCore/page/scrolling/mac/ScrollerPairMac.h:
(WebCore::ScrollerPairMac::protectedNode const):
(WebCore::ScrollerPairMac::node const): Deleted.
* Source/WebCore/page/scrolling/mac/ScrollerPairMac.mm:
(-[WebScrollerImpPairDelegateMac contentAreaRectForScrollerImpPair:]):
(-[WebScrollerImpPairDelegateMac inLiveResizeForScrollerImpPair:]):
(-[WebScrollerImpPairDelegateMac scrollerImpPair:convertContentPoint:toScrollerImp:]):
(-[WebScrollerImpPairDelegateMac scrollerImpPair:updateScrollerStyleForNewRecommendedScrollerStyle:]):
(WebCore::ScrollerPairMac::updateValues):
(WebCore::ScrollerPairMac::visibleSize const):
(WebCore::ScrollerPairMac::useDarkAppearance const):
(WebCore::ScrollerPairMac::scrollbarWidthStyle const):
(WebCore::ScrollerPairMac::valuesForOrientation):
(WebCore::ScrollerPairMac::mouseEnteredContentArea):
(WebCore::ScrollerPairMac::mouseExitedContentArea):

Originally-landed-as: 272448.348 at safari-7618-branch (5572fc8372b9). rdar://124555073
Canonical link: https://commits.webkit.org/276166@main

  Commit: 442482adf1bd2ac184111e7745464cf6a0848b64
      https://github.com/WebKit/WebKit/commit/442482adf1bd2ac184111e7745464cf6a0848b64
  Author: Mark Lam <mark.lam at apple.com>
  Date:   2024-03-15 (Fri, 15 Mar 2024)

  Changed paths:
    A LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map-expected.txt
    A LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map.html
    M Source/WebCore/bindings/js/SerializedScriptValue.cpp

  Log Message:
  -----------
  CachedString::m_jsString is not protected from GC in CloneDeserializer.
https://bugs.webkit.org/show_bug.cgi?id=267971
rdar://120531481

Reviewed by Chris Dumez.

The fix is simply to protect it with the m_keepAliveBuffer.  Also moved the m_keepAliveBuffer from
CloneSerializer to CloneBase.  Previously, I thought that only the serializer needs it.  Now, we
have a case where the deserializer does too.

* LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map-expected.txt: Added.
* LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map.html: Added.
* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneDeserializer::CachedString::jsString):
(WebCore::CloneDeserializer::readTerminal):

Originally-landed-as: 272448.347 at safari-7618-branch (5546b2ee36b5). rdar://124555235
Canonical link: https://commits.webkit.org/276167@main

Compare: https://github.com/WebKit/WebKit/compare/a6406e46b8fd...442482adf1bd

To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications