Changeset 268904 in webkit

Timestamp:

Oct 22, 2020, 9:25:13 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

Null check platformStrategies when making blob read stream for an NSURLRequest
​https://bugs.webkit.org/show_bug.cgi?id=218112
<rdar://problem/70507102>

Patch by Alex Christensen <​achristensen@webkit.org> on 2020-10-22
Reviewed by Wenson Hsieh.

Source/WebCore:

r266187 made it possible to create a DataTransfer without a user event, which allows you to make a FileList,
which allows you to submit a multipart form without a user event that calls decidePolicyForNavigationAction
with a request that has a form data body that would make a blob upload stream. This causes us to use
platformStrategies in the UI process, which dereferences null. Since the blob only really exists in the network
process, just return a nil HTTPBody in such an exotic case instead of crashing.

Covered by an API test that would hit this crash. Web platform tests were insufficient because WebKitTestRunner does not
access WKNavigationAction.request, but Safari does.

• platform/PlatformStrategies.cpp:

(WebCore::hasPlatformStrategies):

• platform/PlatformStrategies.h:
• platform/network/FormData.cpp:

(WebCore::FormData::containsBlobElement const):
(WebCore::FormData::resolveBlobReferences):

• platform/network/FormData.h:
• platform/network/cf/FormDataStreamCFNet.cpp:

(WebCore::createHTTPBodyCFReadStream):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm:

(TEST):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/PlatformStrategies.cpp (modified) (1 diff)
• Source/WebCore/platform/PlatformStrategies.h (modified) (1 diff)
• Source/WebCore/platform/network/FormData.cpp (modified) (1 diff)
• Source/WebCore/platform/network/FormData.h (modified) (1 diff)
• Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-22  Alex Christensen  <achristensen@webkit.org>
+
+        Null check platformStrategies when making blob read stream for an NSURLRequest
+        https://bugs.webkit.org/show_bug.cgi?id=218112
+        <rdar://problem/70507102>
+
+        Reviewed by Wenson Hsieh.
+
+        r266187 made it possible to create a DataTransfer without a user event, which allows you to make a FileList,
+        which allows you to submit a multipart form without a user event that calls decidePolicyForNavigationAction
+        with a request that has a form data body that would make a blob upload stream.  This causes us to use
+        platformStrategies in the UI process, which dereferences null.  Since the blob only really exists in the network
+        process, just return a nil HTTPBody in such an exotic case instead of crashing.
+
+        Covered by an API test that would hit this crash.  Web platform tests were insufficient because WebKitTestRunner does not
+        access WKNavigationAction.request, but Safari does.
+
+        * platform/PlatformStrategies.cpp:
+        (WebCore::hasPlatformStrategies):
+        * platform/PlatformStrategies.h:
+        * platform/network/FormData.cpp:
+        (WebCore::FormData::containsBlobElement const):
+        (WebCore::FormData::resolveBlobReferences):
+        * platform/network/FormData.h:
+        * platform/network/cf/FormDataStreamCFNet.cpp:
+        (WebCore::createHTTPBodyCFReadStream):
+
 2020-10-22  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/platform/PlatformStrategies.cpp

@@ -31 +31 @@
 static PlatformStrategies* s_platformStrategies;
 
+bool hasPlatformStrategies()
+{
+    return !!s_platformStrategies;
+}
+
 PlatformStrategies* platformStrategies()
 {

trunk/Source/WebCore/platform/PlatformStrategies.h

@@ -82 +82 @@
 };
 
+bool hasPlatformStrategies();
 WEBCORE_EXPORT PlatformStrategies* platformStrategies();
 WEBCORE_EXPORT void setPlatformStrategies(PlatformStrategies*);

trunk/Source/WebCore/platform/network/FormData.cpp

@@ -317 +317 @@
 }
 
+bool FormData::containsBlobElement() const
+{
+    for (auto& element : m_elements) {
+        if (WTF::holds_alternative<FormDataElement::EncodedBlobData>(element.data))
+            return true;
+    }
+    return false;
+}
+
 Ref<FormData> FormData::resolveBlobReferences(BlobRegistryImpl* blobRegistryImpl)
 {
     // First check if any blobs needs to be resolved, or we can take the fast path.
-    bool hasBlob = false;
-    for (auto& element : m_elements) {
-        if (WTF::holds_alternative<FormDataElement::EncodedBlobData>(element.data)) {
-            hasBlob = true;
-            break;
-        }
-    }
-
-    if (!hasBlob)
+    if (!containsBlobElement())
         return *this;
 

trunk/Source/WebCore/platform/network/FormData.h

@@ -222 +222 @@
     // If the FormData has no blob references to resolve, this is returned.
     WEBCORE_EXPORT Ref<FormData> resolveBlobReferences(BlobRegistryImpl* = nullptr);
+    bool containsBlobElement() const;
 
     WEBCORE_EXPORT FormDataForUpload prepareForUpload();

trunk/Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp

@@ -32 +32 @@
 #include "BlobRegistryImpl.h"
 #include "FormData.h"
+#include "PlatformStrategies.h"
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -371 +372 @@
 RetainPtr<CFReadStreamRef> createHTTPBodyCFReadStream(FormData& formData)
 {
+    if (!hasPlatformStrategies() && formData.containsBlobElement())
+        return nullptr;
+
     auto resolvedFormData = formData.resolveBlobReferences();
     auto dataForUpload = resolvedFormData->prepareForUpload();

trunk/Tools/ChangeLog

@@ +1 @@
+2020-10-22  Alex Christensen  <achristensen@webkit.org>
+
+        Null check platformStrategies when making blob read stream for an NSURLRequest
+        https://bugs.webkit.org/show_bug.cgi?id=218112
+        <rdar://problem/70507102>
+
+        Reviewed by Wenson Hsieh.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm:
+        (TEST):
+
 2020-10-22  Jonathan Bedard  <jbedard@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm

@@ -26 +26 @@
 #import "config.h"
 
+#import "HTTPServer.h"
 #import "PlatformUtilities.h"
 #import "Test.h"
+#import "TestNavigationDelegate.h"
 #import "TestWKWebView.h"
 #import <WebKit/WKNavigationActionPrivate.h>
@@ -176 +178 @@
     EXPECT_FALSE(navigationDelegate.get().navigationAction._shouldPerformDownload);
 }
+
+TEST(WKNavigationAction, BlobRequestBody)
+{
+    NSString *html = @""
+        "<script>"
+            "function bodyLoaded() {"
+                "const form = Object.assign(document.createElement('form'), {"
+                    "action: '/formAction', method: 'POST', enctype: 'multipart/form-data',"
+                "});"
+                "document.body.append(form);"
+                "const fileInput = Object.assign(document.createElement('input'), {"
+                    "type: 'file', name: 'file',"
+                "});"
+                "form.append(fileInput);"
+                "const dataTransfer = new DataTransfer;"
+                "dataTransfer.items.add(new File(['a'], 'filename'));"
+                "fileInput.files = dataTransfer.files;"
+                "form.submit();"
+            "}"
+        "</script>"
+        "<body onload='bodyLoaded()'>";
+    auto delegate = [[TestNavigationDelegate new] autorelease];
+    auto webView = [[WKWebView new] autorelease];
+    webView.navigationDelegate = delegate;
+    __block bool done = false;
+    delegate.decidePolicyForNavigationAction = ^(WKNavigationAction *action, void (^completionHandler)(WKNavigationActionPolicy)) {
+        EXPECT_NULL(action.request.HTTPBody);
+        if ([action.request.URL.absoluteString isEqualToString:@"about:blank"])
+            completionHandler(WKNavigationActionPolicyAllow);
+        else {
+            EXPECT_WK_STREQ(action.request.URL.absoluteString, "/formAction");
+            completionHandler(WKNavigationActionPolicyCancel);
+            done = true;
+        }
+    };
+    [webView loadHTMLString:html baseURL:nil];
+    TestWebKitAPI::Util::run(&done);
+}