[webkit-changes] [WebKit/WebKit] 2a5083: Web Inspector: UAF needs to be prevented for Remot...

[Robert Jenner]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 2a508351f154cb9a1ff2789d0fceb615f4dd2d7d
      https://github.com/WebKit/WebKit/commit/2a508351f154cb9a1ff2789d0fceb615f4dd2d7d
  Author: Robert Jenner <jenner at apple.com>
  Date:   2024-10-11 (Fri, 11 Oct 2024)

  Changed paths:
    M Source/JavaScriptCore/inspector/remote/RemoteConnectionToTarget.cpp
    M Source/JavaScriptCore/inspector/remote/RemoteConnectionToTarget.h
    M Source/JavaScriptCore/inspector/remote/RemoteControllableTarget.h
    M Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm
    M Source/JavaScriptCore/runtime/JSGlobalObject.cpp
    M Source/JavaScriptCore/runtime/JSGlobalObject.h
    M Source/JavaScriptCore/runtime/JSGlobalObjectDebuggable.cpp
    M Source/JavaScriptCore/runtime/JSGlobalObjectDebuggable.h
    M Source/WebCore/page/Page.cpp
    M Source/WebCore/page/Page.h
    M Source/WebCore/page/PageDebuggable.cpp
    M Source/WebCore/page/PageDebuggable.h
    M Source/WebCore/workers/service/context/ServiceWorkerDebuggable.cpp
    M Source/WebCore/workers/service/context/ServiceWorkerDebuggable.h
    M Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp
    M Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.h
    M Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
    M Source/WebKit/UIProcess/Automation/WebAutomationSession.h
    M Source/WebKit/UIProcess/Inspector/WebPageDebuggable.cpp
    M Source/WebKit/UIProcess/Inspector/WebPageDebuggable.h
    M Source/WebKit/UIProcess/WebPageProxy.cpp
    M Source/WebKit/UIProcess/WebPageProxy.h

  Log Message:
  -----------
  Web Inspector: UAF needs to be prevented for RemoteConnectionToTarget::m_target member variable
https://bugs.webkit.org/show_bug.cgi?id=276192
rdar://129782183

Reviewed by Geoffrey Garen.

`RemoteConnectionToTarget::m_target` was a raw pointer and used from multiple thread.
We have evidence from the radar that `m_target` could be used-after-free in
`RemoteConnectionToTarget::close()`.

To address the issue, I am updating `m_target` to use a ThreadSafeWeakPtr instead
of a raw pointer so that it gets nulled out on target destruction, and still safe
to use from multiple thread.

Of course, using ThreadSafeWeakPtr required updating `RemoteControllableTarget` to
subclass `ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr` and be refcounted. This
required a decent amount of refactoring.

I am also adding WTF_GUARDED_BY_LOCK for the m_target so that the compiler tells
us when we're using it without a lock. It did find a few problems which I fixed.

* Source/JavaScriptCore/inspector/remote/RemoteConnectionToTarget.cpp:
(Inspector::RemoteConnectionToTarget::setup):
(Inspector::RemoteConnectionToTarget::sendMessageToTarget): Deleted.
(Inspector::RemoteConnectionToTarget::close): Deleted.
(Inspector::RemoteConnectionToTarget::targetClosed): Deleted.
(Inspector::RemoteConnectionToTarget::targetIdentifier const): Deleted.
(Inspector::RemoteConnectionToTarget::sendMessageToFrontend): Deleted.
* Source/JavaScriptCore/inspector/remote/RemoteConnectionToTarget.h:
* Source/JavaScriptCore/inspector/remote/RemoteControllableTarget.h:
* Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:
(Inspector::RemoteConnectionToTarget::targetIdentifier const):
(Inspector::RemoteConnectionToTarget::setup):
(Inspector::RemoteConnectionToTarget::close):
(Inspector::RemoteConnectionToTarget::sendMessageToTarget):
(Inspector::RemoteConnectionToTarget::sendMessageToFrontend):
(Inspector::RemoteConnectionToTarget::setupRunLoop):
* Source/JavaScriptCore/runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::~JSGlobalObject):
(JSC::JSGlobalObject::init):
* Source/JavaScriptCore/runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::inspectorDebuggable):
* Source/JavaScriptCore/runtime/JSGlobalObjectDebuggable.cpp:
(JSC::JSGlobalObjectDebuggable::create):
(JSC::JSGlobalObjectDebuggable::JSGlobalObjectDebuggable):
(JSC::JSGlobalObjectDebuggable::name const):
(JSC::JSGlobalObjectDebuggable::connect):
(JSC::JSGlobalObjectDebuggable::disconnect):
(JSC::JSGlobalObjectDebuggable::dispatchMessageFromRemote):
(JSC::JSGlobalObjectDebuggable::pauseWaitingForAutomaticInspection):
(JSC::JSGlobalObjectDebuggable::globalObjectDestroyed):
* Source/JavaScriptCore/runtime/JSGlobalObjectDebuggable.h:
* Source/WebCore/page/Page.cpp:
(WebCore::Page::Page):
(WebCore::Page::~Page):
* Source/WebCore/page/Page.h:
* Source/WebCore/page/PageDebuggable.cpp:
(WebCore::PageDebuggable::create):
(WebCore::PageDebuggable::PageDebuggable):
(WebCore::PageDebuggable::name const):
(WebCore::PageDebuggable::url const):
(WebCore::PageDebuggable::hasLocalDebugger const):
(WebCore::PageDebuggable::connect):
(WebCore::PageDebuggable::disconnect):
(WebCore::PageDebuggable::dispatchMessageFromRemote):
(WebCore::PageDebuggable::setIndicating):
(WebCore::PageDebuggable::detachFromPage):
* Source/WebCore/page/PageDebuggable.h:
* Source/WebCore/workers/service/context/ServiceWorkerDebuggable.cpp:
(WebCore::ServiceWorkerDebuggable::create):
(WebCore::ServiceWorkerDebuggable::connect):
(WebCore::ServiceWorkerDebuggable::disconnect):
(WebCore::ServiceWorkerDebuggable::dispatchMessageFromRemote):
* Source/WebCore/workers/service/context/ServiceWorkerDebuggable.h:
* Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp:
(WebCore::ServiceWorkerThreadProxy::ServiceWorkerThreadProxy):
* Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.h:
* Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::Debuggable::create):
(WebKit::WebAutomationSession::Debuggable::Debuggable):
(WebKit::WebAutomationSession::Debuggable::sessionDestroyed):
(WebKit::WebAutomationSession::Debuggable::name const):
(WebKit::WebAutomationSession::Debuggable::dispatchMessageFromRemote):
(WebKit::WebAutomationSession::Debuggable::connect):
(WebKit::WebAutomationSession::Debuggable::disconnect):
(WebKit::WebAutomationSession::WebAutomationSession):
(WebKit::WebAutomationSession::~WebAutomationSession):
(WebKit::WebAutomationSession::connect):
(WebKit::WebAutomationSession::init):
(WebKit::WebAutomationSession::isPaired const):
(WebKit::WebAutomationSession::isPendingTermination const):
(WebKit::WebAutomationSession::terminate):
* Source/WebKit/UIProcess/Automation/WebAutomationSession.h:
* Source/WebKit/UIProcess/Inspector/WebPageDebuggable.cpp:
(WebKit::WebPageDebuggable::create):
(WebKit::WebPageDebuggable::WebPageDebuggable):
(WebKit::WebPageDebuggable::detachFromPage):
(WebKit::WebPageDebuggable::name const):
(WebKit::WebPageDebuggable::url const):
(WebKit::WebPageDebuggable::hasLocalDebugger const):
(WebKit::WebPageDebuggable::connect):
(WebKit::WebPageDebuggable::disconnect):
(WebKit::WebPageDebuggable::dispatchMessageFromRemote):
(WebKit::WebPageDebuggable::setIndicating):
* Source/WebKit/UIProcess/Inspector/WebPageDebuggable.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::~WebPageProxy):
(WebKit::WebPageProxy::close):
* Source/WebKit/UIProcess/WebPageProxy.h:

Originally-landed-as: 280938.63 at safari-7619-branch (996e25f1fd95). rdar://136111011
Canonical link: https://commits.webkit.org/285039@main



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