[webkit-changes] [WebKit/WebKit] d355b6: Avoid looking up WebKit's NSBundle by identifier

[David Quesada]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: d355b67319be743fa225a00028bffd4ee50a75e4
      https://github.com/WebKit/WebKit/commit/d355b67319be743fa225a00028bffd4ee50a75e4
  Author: David Quesada <david_quesada at apple.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M Source/WebKit/NetworkProcess/mac/NetworkProcessMac.mm
    M Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm
    M Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm
    M Source/WebKit/UIProcess/API/Cocoa/LegacyBundleForClass.mm
    M Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm
    M Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm
    M Source/WebKit/UIProcess/Launcher/cocoa/ProcessLauncherCocoa.mm
    M Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm
    M Source/WebKit/webpushd/WebPushDaemonMain.mm

  Log Message:
  -----------
  Avoid looking up WebKit's NSBundle by identifier
https://bugs.webkit.org/show_bug.cgi?id=257588
rdar://109729179

Reviewed by Chris Dumez.

Some WebKit code that looks up WebKit's NSBundle does so by finding the bundle associated with the
WKWebView class, but most such code does so by looking up the WebKit bundle by its identifier. The
latter approach has a fundamental weakness that the "com.apple.WebKit" NSBundle technically isn't
guaranteed to be a singleton, with one "correct" answer. A process that links a non-system WebKit
has the potential to be sabotaged if stray code in the OS attempts to load the NSBundle at the path
"/System/Library/Frameworks/WebKit.framework" explicitly. NSBundle is happy to return an instance
that can be used to access resources in that system WebKit.framework bundle, and that bundle will
become the result if one looks up the NSBundle for the identifier "com.apple.WebKit". This can have
the awful consequence that web processes will crash on launch due to an alleged mismatch between
the WebKit version observed by the ProcessLauncher and the version observed by the Web Process.

Add hardening to protect against this snafu by reworking all NSBundle lookups to operate based on
+bundleForClass:, like the GPUProcess and `WebProcessProxy::shouldAllowNonValidInjectedCode()`
have been doing. This ensures these call sites will find the NSBundle for the One True WebKit,
since only *one* DYLD image will be loaded into the process even if code tries to dlopen
/S/L/F/WebKit.framework/WebKit.

* Source/WebKit/NetworkProcess/mac/NetworkProcessMac.mm:
(WebKit::NetworkProcess::initializeSandbox):
    Remove a comment that doesn't seem very clear or explain much of what's happening. Presumably
    this comment might have dated back to a time when this file was compiled into the Network
    Process target directly (in which case it would have been true that WebKit.framework is a
    "different" bundle relative to this code), but that is no longer the case.
* Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:
(WebKit::XPCServiceMain):
* Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:
(WebKit::webKit2Bundle):
* Source/WebKit/UIProcess/API/Cocoa/LegacyBundleForClass.mm:
* Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:
(colorForItem):
* Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm:
(WebKit::WebProcessProxy::platformPathsWithAssumedReadAccess):
* Source/WebKit/UIProcess/Launcher/cocoa/ProcessLauncherCocoa.mm:
(WebKit::ProcessLauncher::launchProcess):
* Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:
(WebKit::WebProcess::initializeSandbox):
    Ditto NetworkProcess::initializeSandbox().
* Source/WebKit/webpushd/WebPushDaemonMain.mm:
(WebKit::applySandbox):

Canonical link: https://commits.webkit.org/264801@main