Attachment 372350 Details for Bug 198902 – Patch

[patch] Patch

bug-198902-20190618104438.patch (text/plain), 7.54 KB, created by youenn fablet on 2019-06-18 10:44:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-06-18 10:44:39 PDT

Size: 7.54 KB

patch

obsolete

>Subversion Revision: 246451
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index c65ca91951dba4e9045ce6c66d416bcf2bd9f134..f2aaab43bd8c8e8cf09a8b83f9858a7b1406fd31 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-06-15  Youenn Fablet  <youenn@apple.com>
>+
>+        WebPageProxy should use the right path for sandbox extension
>+        https://bugs.webkit.org/show_bug.cgi?id=198902
>+        rdar://problem/50772810
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When doing process swapping, a file based loading may end up being loaded through loadRequestWithNavigationShared.
>+        In that case, the sandbox extension path is set to '/' which might not be readable by the UIProcess.
>+        If '/' sandbox cannot be created, use a parent directory of the given URL that is inside the root folder of the app
>+        or the given URL otherwise.
>+
>+        Covered by manual testing.
>+
>+        * UIProcess/Cocoa/WebPageProxyCocoa.mm:
>+        (WebKit::WebPageProxy::createSandboxExtensionsIfNeeded):
>+        Make sure we only grant universal access if the sandbox creation succeeds.
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::computeContainerSubdirectoryPath):
>+        (WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle):
>+        (WebKit::WebPageProxy::loadRequestWithNavigationShared):
>+        (WebKit::WebPageProxy::reload):
>+        (WebKit::WebPageProxy::backForwardGoToItemShared):
>+        * UIProcess/WebPageProxy.h:
>+
> 2019-06-14  Youenn Fablet  <youenn@apple.com>
> 
>         WebResourceLoadStatisticsStore should not use its network session if invalidated
>diff --git a/Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm b/Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm
>index a41cd3a004d33b1dad8b6f464f7c5b7b34a120bb..05b6a223d63cbab79eb4a7025dabc6ccaec59475 100644
>--- a/Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm
>@@ -126,8 +126,8 @@ void WebPageProxy::createSandboxExtensionsIfNeeded(const Vector<String>& files,
>     if (files.size() == 1) {
>         BOOL isDirectory;
>         if ([[NSFileManager defaultManager] fileExistsAtPath:files[0] isDirectory:&isDirectory] && !isDirectory) {
>-            SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, fileReadHandle);
>-            willAcquireUniversalFileReadSandboxExtension(m_process);
>+            if (SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, fileReadHandle))
>+                willAcquireUniversalFileReadSandboxExtension(m_process);
>         }
>     }
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 303e74fb73af8abfafda0a4b3b0722ab3c771199..260ca9abd8011c840ddd624ff16c9712e88ca0d4 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -193,6 +193,7 @@
> #include "InsertTextOptions.h"
> #include "RemoteLayerTreeDrawingAreaProxy.h"
> #include "RemoteLayerTreeScrollingPerformanceData.h"
>+#include "SandboxUtilities.h"
> #include "TouchBarMenuData.h"
> #include "TouchBarMenuItemData.h"
> #include "VideoFullscreenManagerProxy.h"
>@@ -1044,19 +1045,43 @@ bool WebPageProxy::tryClose()
>     return false;
> }
> 
>-bool WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& process, const URL& url, SandboxExtension::Handle& sandboxExtensionHandle)
>+static inline String computeContainerSubdirectoryPath(const URL& url)
>+{
>+#if PLATFORM(IOS_FAMILY)
>+    String containerPath = pathForProcessContainer();
>+#else
>+    String containerPath;
>+#endif
>+    if (containerPath.isEmpty())
>+        containerPath = FileSystem::homeDirectoryPath();
>+
>+    auto filePath = url.fileSystemPath();
>+    if (!filePath.startsWith(containerPath))
>+        return filePath;
>+
>+    auto index = filePath.find('/', containerPath.length() + 1);
>+    if (index == notFound)
>+        return filePath;
>+
>+    return filePath.substring(0, index);
>+}
>+
>+void WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& process, const URL& url, SandboxExtension::Handle& sandboxExtensionHandle)
> {
>     if (!url.isLocalFile())
>-        return false;
>+        return;
> 
>     if (process.hasAssumedReadAccessToURL(url))
>-        return false;
>+        return;
> 
>     // Inspector resources are in a directory with assumed access.
>     ASSERT_WITH_SECURITY_IMPLICATION(!WebKit::isInspectorPage(*this));
> 
>-    SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle);
>-    return true;
>+    if (SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) {
>+        willAcquireUniversalFileReadSandboxExtension(process);
>+        return;
>+    }
>+    SandboxExtension::createHandle(computeContainerSubdirectoryPath(url), SandboxExtension::Type::ReadOnly, sandboxExtensionHandle);
> }
> 
> #if !PLATFORM(COCOA)
>@@ -1110,9 +1135,7 @@ void WebPageProxy::loadRequestWithNavigationShared(Ref<WebProcessProxy>&& proces
>     loadParameters.lockHistory = navigation.lockHistory();
>     loadParameters.lockBackForwardList = navigation.lockBackForwardList();
>     loadParameters.clientRedirectSourceForHistory = navigation.clientRedirectSourceForHistory();
>-    bool createdExtension = maybeInitializeSandboxExtensionHandle(process, url, loadParameters.sandboxExtensionHandle);
>-    if (createdExtension)
>-        willAcquireUniversalFileReadSandboxExtension(process);
>+    maybeInitializeSandboxExtensionHandle(process, url, loadParameters.sandboxExtensionHandle);
>     addPlatformLoadParameters(loadParameters);
> 
>     process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
>@@ -1332,9 +1355,7 @@ RefPtr<API::Navigation> WebPageProxy::reload(OptionSet<WebCore::ReloadOption> op
>         m_pageLoadState.setPendingAPIRequestURL(transaction, url);
> 
>         // We may not have an extension yet if back/forward list was reinstated after a WebProcess crash or a browser relaunch
>-        bool createdExtension = maybeInitializeSandboxExtensionHandle(m_process, URL(URL(), url), sandboxExtensionHandle);
>-        if (createdExtension)
>-            willAcquireUniversalFileReadSandboxExtension(m_process);
>+        maybeInitializeSandboxExtensionHandle(m_process, URL(URL(), url), sandboxExtensionHandle);
>     }
> 
>     if (!hasRunningProcess())
>@@ -5731,9 +5752,7 @@ void WebPageProxy::backForwardGoToItemShared(Ref<WebProcessProxy>&& process, con
>         return completionHandler({ });
> 
>     SandboxExtension::Handle sandboxExtensionHandle;
>-    bool createdExtension = maybeInitializeSandboxExtensionHandle(process, URL(URL(), item->url()), sandboxExtensionHandle);
>-    if (createdExtension)
>-        willAcquireUniversalFileReadSandboxExtension(process);
>+    maybeInitializeSandboxExtensionHandle(process, URL(URL(), item->url()), sandboxExtensionHandle);
>     m_backForwardList->goToItem(*item);
>     completionHandler(WTFMove(sandboxExtensionHandle));
> }
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index f54f67fe486ef1899b1328755cda0788f1cdb3da..0cf4962651c90c695add505bb779776a9d97df59 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1898,7 +1898,7 @@ private:
>     void setPluginComplexTextInputState(uint64_t pluginComplexTextInputIdentifier, uint64_t complexTextInputState);
> #endif
> 
>-    bool maybeInitializeSandboxExtensionHandle(WebProcessProxy&, const URL&, SandboxExtension::Handle&);
>+    void maybeInitializeSandboxExtensionHandle(WebProcessProxy&, const URL&, SandboxExtension::Handle&);
> 
> #if USE(AUTOMATIC_TEXT_REPLACEMENT)
>     void toggleSmartInsertDelete();

Actions: View | Formatted Diff | Diff

Attachments on bug 198902: 372216 | 372218 | 372219 | 372275 | 372340 | 372350 | 372513 | 372556 | 372574