[PSON] Unable to submit a file in FormData cross-site
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Nov 2018 17:09:56 +0000 (17:09 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Nov 2018 17:09:56 +0000 (17:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191138

Reviewed by Alex Christensen.

Source/WebKit:

When PSON is enabled, we are unable to submit a file in FormData cross-site. Although we encode the
request body over IPC since r237639, we're missing the right sandbox extensions for its to work for
files.

Update FormDataReference encoder to pass along the necessary sandbox extensions for files in the
FormData, and have its decoder consume those extensions so that the recipient has access to those
files. Also update LoadParameters's IPC encoder / decoder to encoder an IPC::FormDataReference
(which encodes both FormData and sandbox extensions) instead of a FormData.

* Platform/IPC/FormDataReference.h:
(IPC::FormDataReference::encode const):
(IPC::FormDataReference::decode):
* Shared/LoadParameters.cpp:
(WebKit::LoadParameters::encode const):
(WebKit::LoadParameters::decode):

LayoutTests:

Add layout test coverage.

* http/tests/misc/form-submit-file-cross-site-expected.txt:
* http/tests/misc/form-submit-file-cross-site.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237683 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/misc/form-submit-file-cross-site-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/misc/form-submit-file-cross-site.html [new file with mode: 0644]
LayoutTests/http/tests/misc/resources/check-test-file.php [new file with mode: 0644]
LayoutTests/platform/win/TestExpectations
Source/WebKit/ChangeLog
Source/WebKit/Platform/IPC/FormDataReference.h
Source/WebKit/Shared/LoadParameters.cpp

index 0956316..347458c 100644 (file)
@@ -1,3 +1,15 @@
+2018-11-01  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] Unable to submit a file in FormData cross-site
+        https://bugs.webkit.org/show_bug.cgi?id=191138
+
+        Reviewed by Alex Christensen.
+
+        Add layout test coverage.
+
+        * http/tests/misc/form-submit-file-cross-site-expected.txt:
+        * http/tests/misc/form-submit-file-cross-site.html:
+
 2018-11-01  Devin Rousso  <drousso@apple.com>
 
         Unreviewed test fix after r237670.
diff --git a/LayoutTests/http/tests/misc/form-submit-file-cross-site-expected.txt b/LayoutTests/http/tests/misc/form-submit-file-cross-site-expected.txt
new file mode 100644 (file)
index 0000000..05ed762
--- /dev/null
@@ -0,0 +1,7 @@
+OPEN FILE PANEL
+This tests verifies the test file included in the multipart form data:
+
+PASS: File is present
+PASS: File upload was successful
+PASS: File size is greater than 0
+
diff --git a/LayoutTests/http/tests/misc/form-submit-file-cross-site.html b/LayoutTests/http/tests/misc/form-submit-file-cross-site.html
new file mode 100644 (file)
index 0000000..c2a480a
--- /dev/null
@@ -0,0 +1,41 @@
+<html>
+<head>
+<script src="/js-test-resources/ui-helper.js"></script>
+<script src="/local/fileapi/resources/temp-file-utils.js"></script>
+<script>
+
+const tempFileContent = "TEST";
+const tempFileName = "test.tmp";
+
+function onChange()
+{
+    document.forms[0].submit();
+}
+
+function test()
+{
+    let tempFilePath = createTempFile(tempFileName, tempFileContent);
+
+    let input = document.getElementsByTagName("input")[0];
+    input.onchange = onChange;
+
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+        testRunner.setOpenPanelFiles([tempFilePath]);
+
+        var centerX = input.offsetLeft + input.offsetWidth / 2;
+        var centerY = input.offsetTop + input.offsetHeight / 2;
+        UIHelper.activateAt(centerX, centerY);
+    }
+}
+
+window.onload = test;
+</script>
+</head>
+<body>
+<form enctype="multipart/form-data" method="POST" action="http://localhost:8000/misc/resources/check-test-file.php">
+<input name="data" type="file"></input>
+</form>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/misc/resources/check-test-file.php b/LayoutTests/http/tests/misc/resources/check-test-file.php
new file mode 100644 (file)
index 0000000..cd952ee
--- /dev/null
@@ -0,0 +1,34 @@
+<?php
+header("Content-Type: text/html; charset=UTF-8");
+?>
+<html>
+<head>
+<script>
+onload = function() {
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+</script>
+</head>
+<body>
+<h1>This tests verifies the test file included in the multipart form data:</h1>
+<pre>
+<?php
+if (isset($_FILES['data'])) {
+    print "PASS: File is present\n";
+    if ($_FILES['data']['error'] == UPLOAD_ERR_OK)
+        print "PASS: File upload was successful\n";
+    else
+        print "FAIL: File upload failed\n";
+
+    if ($_FILES['data']['size'] > 0)
+        print "PASS: File size is greater than 0\n";
+    else
+        print "FAIL: File size is 0\n";
+} else {
+    print "FAIL: File is missing";
+}
+?>
+</pre>
+</body>
+</html>
index ae0bfed..5ba8f37 100644 (file)
@@ -209,6 +209,7 @@ http/tests/xmlhttprequest/upload-onprogress-event.html [ Skip ]
 fast/files [ Skip ]
 http/tests/fileapi [ Skip ]
 fast/forms/file/recover-file-input-in-unposted-form.html [ Skip ]
+http/tests/misc/form-submit-file-cross-site.html [ Skip ]
 http/tests/xmlhttprequest/post-blob-content-type-async.html [ Skip ]
 http/tests/xmlhttprequest/post-blob-content-type-sync.html [ Skip ]
 webkit.org/b/29287 http/tests/local/fileapi/ [ Skip ]
index d754b29..d702b04 100644 (file)
@@ -1,3 +1,26 @@
+2018-11-01  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] Unable to submit a file in FormData cross-site
+        https://bugs.webkit.org/show_bug.cgi?id=191138
+
+        Reviewed by Alex Christensen.
+
+        When PSON is enabled, we are unable to submit a file in FormData cross-site. Although we encode the
+        request body over IPC since r237639, we're missing the right sandbox extensions for its to work for
+        files.
+
+        Update FormDataReference encoder to pass along the necessary sandbox extensions for files in the
+        FormData, and have its decoder consume those extensions so that the recipient has access to those
+        files. Also update LoadParameters's IPC encoder / decoder to encoder an IPC::FormDataReference
+        (which encodes both FormData and sandbox extensions) instead of a FormData.
+
+        * Platform/IPC/FormDataReference.h:
+        (IPC::FormDataReference::encode const):
+        (IPC::FormDataReference::decode):
+        * Shared/LoadParameters.cpp:
+        (WebKit::LoadParameters::encode const):
+        (WebKit::LoadParameters::decode):
+
 2018-11-01  Claudio Saavedra  <csaavedra@igalia.com>
 
         ERROR: ResourceLoadStatisticsPersistentStorage: Unable to delete statistics file
index a6d2ea8..a4d89bc 100644 (file)
 
 #include "Decoder.h"
 #include "Encoder.h"
+#include "SandboxExtension.h"
 #include <WebCore/FormData.h>
 
 namespace IPC {
 
-// FIXME: In case of file based form data, we should pass sandbox extensions as well.
 class FormDataReference {
 public:
     FormDataReference() = default;
@@ -45,8 +45,26 @@ public:
     void encode(Encoder& encoder) const
     {
         encoder << !!m_data;
-        if (m_data)
-            encoder << *m_data;
+        if (!m_data)
+            return;
+
+        encoder << *m_data;
+
+        auto& elements = m_data->elements();
+        size_t fileCount = std::count_if(elements.begin(), elements.end(), [](auto& element) {
+            return WTF::holds_alternative<WebCore::FormDataElement::EncodedFileData>(element.data);
+        });
+
+        WebKit::SandboxExtension::HandleArray sandboxExtensionHandles;
+        sandboxExtensionHandles.allocate(fileCount);
+        size_t extensionIndex = 0;
+        for (auto& element : elements) {
+            if (auto* fileData = WTF::get_if<WebCore::FormDataElement::EncodedFileData>(element.data)) {
+                const String& path = fileData->shouldGenerateFile ? fileData->generatedFilename : fileData->filename;
+                WebKit::SandboxExtension::createHandle(path, WebKit::SandboxExtension::Type::ReadOnly, sandboxExtensionHandles[extensionIndex++]);
+            }
+        }
+        encoder << sandboxExtensionHandles;
     }
 
     static std::optional<FormDataReference> decode(Decoder& decoder)
@@ -62,6 +80,14 @@ public:
         if (!formData)
             return std::nullopt;
 
+        std::optional<WebKit::SandboxExtension::HandleArray> sandboxExtensionHandles;
+        decoder >> sandboxExtensionHandles;
+        if (!sandboxExtensionHandles)
+            return std::nullopt;
+
+        for (size_t i = 0; i < sandboxExtensionHandles->size(); ++i)
+            WebKit::SandboxExtension::consumePermanently(sandboxExtensionHandles->at(i));
+
         return FormDataReference { formData.releaseNonNull() };
     }
 
index beb6244..e4fcb02 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "LoadParameters.h"
 
+#include "FormDataReference.h"
 #include "WebCoreArgumentCoders.h"
 
 namespace WebKit {
@@ -36,8 +37,10 @@ void LoadParameters::encode(IPC::Encoder& encoder) const
     encoder << request;
 
     encoder << static_cast<bool>(request.httpBody());
-    if (request.httpBody())
-        request.httpBody()->encode(encoder);
+    if (request.httpBody()) {
+        // FormDataReference encoder / decoder takes care of passing and consuming the needed sandbox extensions.
+        encoder << IPC::FormDataReference { request.httpBody() };
+    }
 
     encoder << sandboxExtensionHandle;
     encoder << data;
@@ -70,10 +73,13 @@ bool LoadParameters::decode(IPC::Decoder& decoder, LoadParameters& data)
         return false;
 
     if (hasHTTPBody) {
-        RefPtr<WebCore::FormData> formData = WebCore::FormData::decode(decoder);
-        if (!formData)
+        // FormDataReference encoder / decoder takes care of passing and consuming the needed sandbox extensions.
+        std::optional<IPC::FormDataReference> formDataReference;
+        decoder >> formDataReference;
+        if (!formDataReference)
             return false;
-        data.request.setHTTPBody(WTFMove(formData));
+
+        data.request.setHTTPBody(formDataReference->takeData());
     }
 
     std::optional<SandboxExtension::Handle> sandboxExtensionHandle;