Changeset 222688 in webkit

Timestamp:

Sep 30, 2017 11:59:00 PM (7 years ago)

Author:

rniwa@webkit.org

Message:

Don't reveal file URL when pasting an image
​https://bugs.webkit.org/show_bug.cgi?id=177710
<rdar://problem/34757924>

Reviewed by Wenson Hsieh.

Source/WebCore:

Fixed the bug by generalizing the code we had for drag & drop to hide string types when there is a file.

We don't hide string types when customPasteboardDataEnabled() is false to preserve the backwards compatiblity
with apps that are relying on being able to read files URLs in the pasteboard.

Test: editing/pasteboard/paste-image-does-not-reveal-file-url.html

• dom/DataTransfer.cpp:

(WebCore::DataTransfer::getData const): Pretend there is no string data when there is a file in the pasteboard
custom pasteboard data is enabled.
(WebCore::DataTransfer::setData): Ditto.
(WebCore::DataTransfer::types const): Ditto.

• dom/DataTransfer.h:

(WebCore::DataTransfer::forDrag const): Added for when drag & drop support is disabled at compilation time.
(WebCore::DataTransfer::forFileDrag const): Ditto.

• platform/Pasteboard.h:
• platform/StaticPasteboard.h:
• platform/cocoa/PasteboardCocoa.mm:

(WebCore::Pasteboard::containsFiles): Added.

• platform/gtk/PasteboardGtk.cpp:

(WebCore::Pasteboard::containsFiles): Added.

• platform/win/PasteboardWin.cpp:

(WebCore::Pasteboard::containsFiles): Added.

• platform/wpe/PasteboardWPE.cpp:

(WebCore::Pasteboard::containsFiles): Added.
(WebCore::Pasteboard::readFilenames): Annotated this function with notImplemented().

LayoutTests:

Added a regression test for pasting an image. We enable this protection only when custom data is enabled
to preserve the backwards compatibility.

• editing/pasteboard/paste-image-does-not-reveal-file-url-expected.txt: Added.
• editing/pasteboard/paste-image-does-not-reveal-file-url.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/pasteboard/paste-image-does-not-reveal-file-url-expected.txt (added)
• LayoutTests/editing/pasteboard/paste-image-does-not-reveal-file-url.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/DataTransfer.cpp (modified) (3 diffs)
• Source/WebCore/dom/DataTransfer.h (modified) (1 diff)
• Source/WebCore/platform/Pasteboard.h (modified) (1 diff)
• Source/WebCore/platform/StaticPasteboard.h (modified) (1 diff)
• Source/WebCore/platform/cocoa/PasteboardCocoa.mm (modified) (1 diff)
• Source/WebCore/platform/gtk/PasteboardGtk.cpp (modified) (1 diff)
• Source/WebCore/platform/win/PasteboardWin.cpp (modified) (1 diff)
• Source/WebCore/platform/wpe/PasteboardWPE.cpp (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-09-30  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Don't reveal file URL when pasting an image
+        https://bugs.webkit.org/show_bug.cgi?id=177710
+        <rdar://problem/34757924>
+
+        Reviewed by Wenson Hsieh.
+
+        Added a regression test for pasting an image. We enable this protection only when custom data is enabled
+        to preserve the backwards compatibility.
+
+        * editing/pasteboard/paste-image-does-not-reveal-file-url-expected.txt: Added.
+        * editing/pasteboard/paste-image-does-not-reveal-file-url.html: Added.
+
 2017-09-30  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-09-30  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Don't reveal file URL when pasting an image
+        https://bugs.webkit.org/show_bug.cgi?id=177710
+        <rdar://problem/34757924>
+
+        Reviewed by Wenson Hsieh.
+
+        Fixed the bug by generalizing the code we had for drag & drop to hide string types when there is a file.
+
+        We don't hide string types when customPasteboardDataEnabled() is false to preserve the backwards compatiblity
+        with apps that are relying on being able to read files URLs in the pasteboard.
+
+        Test: editing/pasteboard/paste-image-does-not-reveal-file-url.html
+
+        * dom/DataTransfer.cpp:
+        (WebCore::DataTransfer::getData const): Pretend there is no string data when there is a file in the pasteboard
+        custom pasteboard data is enabled.
+        (WebCore::DataTransfer::setData): Ditto.
+        (WebCore::DataTransfer::types const): Ditto.
+        * dom/DataTransfer.h:
+        (WebCore::DataTransfer::forDrag const): Added for when drag & drop support is disabled at compilation time.
+        (WebCore::DataTransfer::forFileDrag const): Ditto.
+        * platform/Pasteboard.h:
+        * platform/StaticPasteboard.h:
+        * platform/cocoa/PasteboardCocoa.mm:
+        (WebCore::Pasteboard::containsFiles): Added.
+        * platform/gtk/PasteboardGtk.cpp:
+        (WebCore::Pasteboard::containsFiles): Added.
+        * platform/win/PasteboardWin.cpp:
+        (WebCore::Pasteboard::containsFiles): Added.
+        * platform/wpe/PasteboardWPE.cpp:
+        (WebCore::Pasteboard::containsFiles): Added.
+        (WebCore::Pasteboard::readFilenames): Annotated this function with notImplemented().
+
 2017-09-30  Sam Weinig  <sam@webkit.org>
 

trunk/Source/WebCore/dom/DataTransfer.cpp

@@ -138 +138 @@
         return String();
 
-#if ENABLE(DRAG_SUPPORT)
-    if (forFileDrag() && m_pasteboard->readFilenames().size())
+    if ((forFileDrag() || Settings::customPasteboardDataEnabled()) && m_pasteboard->containsFiles())
         return { };
-#endif
 
     auto normalizedType = normalizeType(type);
@@ -154 +152 @@
         return;
 
-#if ENABLE(DRAG_SUPPORT)
-    if (forFileDrag() && m_pasteboard->readFilenames().size())
-        return;
-#endif
+    if ((forFileDrag() || Settings::customPasteboardDataEnabled()) && m_pasteboard->containsFiles())
+        return;
 
     String normalizedType = normalizeType(type);
@@ -176 +172 @@
     if (!canReadTypes())
         return { };
-
-    return Settings::customPasteboardDataEnabled() ? m_pasteboard->typesSafeForBindings() : m_pasteboard->typesForLegacyUnsafeBindings();
+    
+    if (!Settings::customPasteboardDataEnabled())
+        return m_pasteboard->typesForLegacyUnsafeBindings();
+
+    if (m_pasteboard->containsFiles())
+        return { "Files" };
+    return m_pasteboard->typesSafeForBindings();
 }
 

trunk/Source/WebCore/dom/DataTransfer.h

@@ -107 +107 @@
     bool forDrag() const { return m_type == Type::DragAndDropData || m_type == Type::DragAndDropFiles; }
     bool forFileDrag() const { return m_type == Type::DragAndDropFiles; }
+#else
+    bool forDrag() const { return false; }
+    bool forFileDrag() const { return false; }
 #endif
 

trunk/Source/WebCore/platform/Pasteboard.h

@@ -216 +216 @@
     virtual void write(const PasteboardWebContent&);
 
+    virtual bool containsFiles();
     virtual Vector<String> readFilenames();
     virtual bool canSmartReplace();

trunk/Source/WebCore/platform/StaticPasteboard.h

@@ -59 +59 @@
     void write(const PasteboardWebContent&) final { }
 
+    bool containsFiles() final { return false; }
     Vector<String> readFilenames() final { return { }; }
     bool canSmartReplace() final { return false; }

trunk/Source/WebCore/platform/cocoa/PasteboardCocoa.mm

@@ -152 +152 @@
 }
 
+bool Pasteboard::containsFiles()
+{
+    if (!platformStrategies()->pasteboardStrategy()->getNumberOfFiles(m_pasteboardName)) {
+        Vector<String> cocoaTypes;
+        platformStrategies()->pasteboardStrategy()->getTypes(cocoaTypes, m_pasteboardName);
+        if (cocoaTypes.findMatching([](const String& cocoaType) { return shouldTreatCocoaTypeAsFile(cocoaType); }) == notFound)
+            return false;
+    }
+
+    // Enforce changeCount ourselves for security. We check after reading instead of before to be
+    // sure it doesn't change between our testing the change count and accessing the data.
+    if (m_changeCount != platformStrategies()->pasteboardStrategy()->changeCount(m_pasteboardName))
+        return false;
+
+    return true;
+}
+
 Vector<String> Pasteboard::typesSafeForBindings()
 {

trunk/Source/WebCore/platform/gtk/PasteboardGtk.cpp

@@ -320 +320 @@
 }
 
+bool Pasteboard::containsFiles()
+{
+    return readFilenames().size();
+}
+
 Vector<String> Pasteboard::readFilenames()
 {

trunk/Source/WebCore/platform/win/PasteboardWin.cpp

@@ -313 +313 @@
     notImplemented();
     return { };
+}
+
+bool Pasteboard::containsFiles()
+{
+    return readFilenames().size(); // FIXME: Make this code more efficient.
 }
 

trunk/Source/WebCore/platform/wpe/PasteboardWPE.cpp

@@ -125 +125 @@
 }
 
+bool Pasteboard::containsFiles()
+{
+    notImplemented();
+    return false;
+}
+
 Vector<String> Pasteboard::readFilenames()
 {
+    notImplemented();
     return Vector<String>();
 }

trunk/Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm

@@ -1037 +1037 @@
 {
     NSArray<NSString *> *expectedOutput = @[
-        @"Found data transfer item (kind: 'string', type: 'text/plain')",
         @"Found data transfer item (kind: 'file', type: 'text/plain')",
         @"FILE: /foo.txt ('text/plain', 28 bytes)"
@@ -1558 +1557 @@
     // File URLs should never be exposed directly to web content, so DataTransfer.getData should return an empty string here.
     checkJSONWithLogging([webView stringByEvaluatingJavaScript:@"output.value"], @{
-        @"dragover": @{ @"Files": @"", @"text/uri-list" : @"" },
-        @"drop": @{ @"Files": @"", @"text/uri-list" : @"" }
+        @"dragover": @{ @"Files": @"" },
+        @"drop": @{ @"Files": @"" }
     });
 }
@@ -1580 +1579 @@
         [simulator runFrom:CGPointMake(300, 375) to:CGPointMake(50, 375)];
         checkJSONWithLogging([webView stringByEvaluatingJavaScript:@"output.value"], @{
-            @"dragover": @{ @"text/plain" : @"" },
-            @"drop": @{ @"text/plain" : @"" }
+            @"dragover": @{ @"Files" : @"" },
+            @"drop": @{ @"Files" : @"" }
         });