Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: c8d323b1851ec55adde59e1c5ccaa61d9effc0a9
https://github.com/WebKit/WebKit/commit/c8d323b1851ec55adde59e1c5ccaa61d9effc0a9
Author: Kiet Ho <[email protected]>
Date: 2024-11-04 (Mon, 04 Nov 2024)
Changed paths:
A LayoutTests/fast/dom/view-transition-lifetime-crash-expected.txt
A LayoutTests/fast/dom/view-transition-lifetime-crash.html
M Source/WebCore/Modules/screen-wake-lock/WakeLockManager.cpp
M Source/WebCore/Modules/screen-wake-lock/WakeLockManager.h
M Source/WebCore/Modules/screen-wake-lock/WakeLockSentinel.cpp
M Source/WebCore/dom/Document.cpp
M Source/WebCore/dom/ViewTransition.cpp
M Source/WebCore/dom/ViewTransition.h
M Source/WebCore/dom/VisibilityChangeClient.h
Log Message:
-----------
REGRESSION (283084@main): Document::visibilityStateChanged does not hold
reference to callback clients
rdar://138799302
https://bugs.webkit.org/show_bug.cgi?id=282360
Reviewed by Tim Nguyen, Ryosuke Niwa, and Chris Dumez.
Document::visibilityStateChanged() invokes visibility state callback clients,
but does not
hold a reference to them before invoking. The client could then accidentally
free itself
and cause an UAF. One possible route that leads to an UAF is through
ViewTransition,
which the test case demonstrates:
* The ViewTransition C++ objects are allocated by
document.startViewTransition().
After the call, each object has a ref count of at least 2 (one in the JS
wrapper
that wraps the C++ object, one in Document::m_activeViewTransition)
* The GC is invoked, which releases the JS wrappers and decreases the ref count
to 1
* The document visibility state is changed. This invokes
ViewTransition::visibilityStateChanged
on each object, which calls ::skipViewTransition, which calls
::clearViewTransition.
::clearViewTransition sets Document::m_activeViewTransition to null, so the
object ref
count is 0 and it's deallocated. ::clearViewTransition then continues to
modify the
(already deallocated) object, leading to an UAF.
Fix this by holding a reference to the callback clients before invoking it.
This involves
making VisibilityChangeClient ref counted. Then
Document::visibilityStateChanged()
would hold a reference to the client before invoking it. As WakeLockManager
(which inherits VisibilityChangeClient) wasn't ref counted, this patch also
makes it
ref counted.
It's also observed that the JS wrapper should not be deallocated by the GC
before the
view transition has completed. This commit fixes this by implementing
ViewTransition::virtualHasPendingActivity(), which the GC consults to determine
whether
to deallocate the wrapper or not.
* LayoutTests/fast/dom/view-transition-lifetime-crash-expected.txt: Added.
* LayoutTests/fast/dom/view-transition-lifetime-crash.html: Added.
* Source/WebCore/Modules/screen-wake-lock/WakeLockManager.cpp:
(WebCore::WakeLockManager::ref const): Delegated ref() to the document.
(WebCore::WakeLockManager::deref const): Delegated deref() to the document.
* Source/WebCore/Modules/screen-wake-lock/WakeLockManager.h: Made
WakeLockManager ref counted by declaring ref() and deref().
* Source/WebCore/Modules/screen-wake-lock/WakeLockSentinel.cpp:
(WebCore::WakeLockSentinel::release): Hold a reference to the document's
WakeLockManager before using it.
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::visibilityStateChanged): Hold a reference to the visibility
state callback client before calling it.
(WebCore::Document::wakeLockManager): Used makeUniqueWithoutRefCountedCheck to
create new WakeLockManager.
* Source/WebCore/dom/ViewTransition.cpp:
(WebCore::ViewTransition::virtualHasPendingActivity const): Added
implementation.
* Source/WebCore/dom/ViewTransition.h:
* Source/WebCore/dom/VisibilityChangeClient.h: Made VisibilityChangeClient ref
counted.
Canonical link: https://commits.webkit.org/286136@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
