Changeset 273271 in webkit

Timestamp:

Feb 22, 2021 1:08:13 PM (3 years ago)

Author:

pvollan@apple.com

Message:

[macOS] Crash under AuxiliaryProcess::initializeSandbox
​https://bugs.webkit.org/show_bug.cgi?id=222233
<rdar://problem/74261611>

Reviewed by Brent Fulgham.

When a WebKit client provides a user directory suffix in the process initialization parameters, confstr with the new
user suffix applied will fail to create the full directory path if it does not exist, and return an empty result.
This will lead to empty paths in the sandbox parameters, which will cause the sandbox to fail to compile, which will
eventually crash the WebKit process. This patch addresses this by making sure the new user directory suffix does not
represent a path, since confstr is not able to handle user directory suffixes containing paths that do not exist.
Additionally, this patch reverts r271417, which was the first attempt at fixing this crash, but was unsuccessful in
doing so.

• Shared/mac/AuxiliaryProcessMac.mm:

(WebKit::initializeSandboxParameters):
(WebKit::AuxiliaryProcess::initializeSandbox):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Shared/mac/AuxiliaryProcessMac.mm (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-02-22  Per Arne Vollan  <pvollan@apple.com>
+
+        [macOS] Crash under AuxiliaryProcess::initializeSandbox
+        https://bugs.webkit.org/show_bug.cgi?id=222233
+        <rdar://problem/74261611>
+
+        Reviewed by Brent Fulgham.
+
+        When a WebKit client provides a user directory suffix in the process initialization parameters, confstr with the new
+        user suffix applied will fail to create the full directory path if it does not exist, and return an empty result.
+        This will lead to empty paths in the sandbox parameters, which will cause the sandbox to fail to compile, which will
+        eventually crash the WebKit process. This patch addresses this by making sure the new user directory suffix does not
+        represent a path, since confstr is not able to handle user directory suffixes containing paths that do not exist.
+        Additionally, this patch reverts r271417, which was the first attempt at fixing this crash, but was unsuccessful in
+        doing so.
+
+        * Shared/mac/AuxiliaryProcessMac.mm:
+        (WebKit::initializeSandboxParameters):
+        (WebKit::AuxiliaryProcess::initializeSandbox):
+
 2021-02-22  Per Arne  <pvollan@apple.com>
 

trunk/Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm

@@ -640 +640 @@
     if (sandboxParameters.userDirectorySuffix().isNull()) {
         auto userDirectorySuffix = parameters.extraInitializationData.find("user-directory-suffix");
-        if (userDirectorySuffix != parameters.extraInitializationData.end())
-            sandboxParameters.setUserDirectorySuffix([makeString(userDirectorySuffix->value, '/', String([[NSBundle mainBundle] bundleIdentifier])) fileSystemRepresentation]);
-        else {
+        if (userDirectorySuffix != parameters.extraInitializationData.end()) {
+            String suffix = userDirectorySuffix->value;
+            auto firstPathSeparator = suffix.find("/");
+            if (firstPathSeparator != notFound)
+                suffix.truncate(firstPathSeparator);
+            sandboxParameters.setUserDirectorySuffix(suffix);
+        } else {
             String clientIdentifier = codeSigningIdentifier(parameters.connectionIdentifier.xpcConnection.get());
             if (clientIdentifier.isNull())
@@ -661 +665 @@
 
     // Use private temporary and cache directories.
-    _set_user_dir_suffix(FileSystem::fileSystemRepresentation(sandboxParameters.userDirectorySuffix()).data());
+    setenv("DIRHELPER_USER_DIR_SUFFIX", FileSystem::fileSystemRepresentation(sandboxParameters.userDirectorySuffix()).data(), 1);
     char temporaryDirectory[PATH_MAX];
     if (!confstr(_CS_DARWIN_USER_TEMP_DIR, temporaryDirectory, sizeof(temporaryDirectory))) {
@@ -707 +711 @@
     // This must be called before initializeSandboxParameters so that the path does not include the user directory suffix.
     // We don't want the user directory suffix because we want all processes of the same type to use the same cache directory.
-    // First, make sure the user directory suffix is empty at this point. This is normally already the case, but some host
-    // processes are setting the user directory suffix, which we will inherit, and our sandbox datavault will then end up
-    // inside the host process' cache folder, which is undesirable.
-    _set_user_dir_suffix(nullptr);
     String dataVaultParentDirectory { sandboxDataVaultParentDirectory() };
 #else