[macOS] Avoid crashing the UI process when writing empty data to the pasteboard
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 May 2019 14:39:10 +0000 (14:39 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 May 2019 14:39:10 +0000 (14:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197644
<rdar://problem/50526364>

Reviewed by Tim Horton.

Source/WebKit:

Test: WebKit.WKWebProcessPlugInDoNotCrashWhenCopyingEmptyClientData

* WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
(WebKit::WebPlatformStrategies::setBufferForType):

Make this function robust by not attempting to create a shared memory buffer in the case where the given data
buffer is empty.

Tools:

Add a new API test to exercise a possible scenario where we may crash while writing data to the pasteboard.

* TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegate.mm:
* TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegatePlugIn.mm:
(-[BundleEditingDelegatePlugIn webProcessPlugIn:didCreateBrowserContextController:]):
(-[BundleEditingDelegatePlugIn _webProcessPlugInBrowserContextController:pasteboardDataForRange:]):

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

Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegate.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegatePlugIn.mm

index 04e2116..88ce45e 100644 (file)
@@ -1,3 +1,19 @@
+2019-05-07  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [macOS] Avoid crashing the UI process when writing empty data to the pasteboard
+        https://bugs.webkit.org/show_bug.cgi?id=197644
+        <rdar://problem/50526364>
+
+        Reviewed by Tim Horton.
+
+        Test: WebKit.WKWebProcessPlugInDoNotCrashWhenCopyingEmptyClientData
+
+        * WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
+        (WebKit::WebPlatformStrategies::setBufferForType):
+
+        Make this function robust by not attempting to create a shared memory buffer in the case where the given data
+        buffer is empty.
+
 2019-05-07  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Crash in webkitWebViewBaseSetEnableBackForwardNavigationGesture
index 3466624..0668167 100644 (file)
@@ -202,7 +202,7 @@ long WebPlatformStrategies::setTypes(const Vector<String>& pasteboardTypes, cons
 long WebPlatformStrategies::setBufferForType(SharedBuffer* buffer, const String& pasteboardType, const String& pasteboardName)
 {
     SharedMemory::Handle handle;
-    if (buffer) {
+    if (buffer && buffer->size()) {
         RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::allocate(buffer->size());
         // FIXME: Null check prevents crashing, but it is not great that we will have empty pasteboard content for this type,
         // because we've already set the types.
index 4fe89f7..53c2cb6 100644 (file)
@@ -1,3 +1,18 @@
+2019-05-07  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [macOS] Avoid crashing the UI process when writing empty data to the pasteboard
+        https://bugs.webkit.org/show_bug.cgi?id=197644
+        <rdar://problem/50526364>
+
+        Reviewed by Tim Horton.
+
+        Add a new API test to exercise a possible scenario where we may crash while writing data to the pasteboard.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegate.mm:
+        * TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegatePlugIn.mm:
+        (-[BundleEditingDelegatePlugIn webProcessPlugIn:didCreateBrowserContextController:]):
+        (-[BundleEditingDelegatePlugIn _webProcessPlugInBrowserContextController:pasteboardDataForRange:]):
+
 2019-05-07  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK][WPE] MiniBrowser: load about:blank for new web views in automation mode
index eb982bc..c5fca3e 100644 (file)
@@ -115,4 +115,21 @@ TEST(WebKit, WKWebProcessPlugInEditingDelegate)
     TestWebKitAPI::Util::run(&doneEvaluatingJavaScript);
 }
 
-#endif
+TEST(WebKit, WKWebProcessPlugInDoNotCrashWhenCopyingEmptyClientData)
+{
+    auto configuration = retainPtr([WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"BundleEditingDelegatePlugIn"]);
+    [[configuration processPool] _setObject:@YES forBundleParameter:@"EditingDelegateShouldWriteEmptyData"];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    [webView loadHTMLString:@"<body style='-webkit-user-modify: read-write-plaintext-only'>Just something to copy <script> var textNode = document.body.firstChild; document.getSelection().setBaseAndExtent(textNode, 5, textNode, 14) </script>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+
+    auto object = adoptNS([[BundleEditingDelegateRemoteObject alloc] init]);
+    _WKRemoteObjectInterface *interface = [_WKRemoteObjectInterface remoteObjectInterfaceWithProtocol:@protocol(BundleEditingDelegateProtocol)];
+    [[webView _remoteObjectRegistry] registerExportedObject:object.get() interface:interface];
+
+    [webView performSelector:@selector(copy:) withObject:nil];
+    TestWebKitAPI::Util::run(&didWriteToPasteboard);
+}
+
+#endif // PLATFORM(MAC)
index 9231484..8f16997 100644 (file)
@@ -45,6 +45,7 @@
     RetainPtr<id <BundleEditingDelegateProtocol>> _remoteObject;
     BOOL _editingDelegateShouldInsertText;
     BOOL _shouldOverridePerformTwoStepDrop;
+    BOOL _shouldWriteEmptyData;
 }
 
 - (void)webProcessPlugIn:(WKWebProcessPlugInController *)plugInController didCreateBrowserContextController:(WKWebProcessPlugInBrowserContextController *)browserContextController
@@ -60,6 +61,7 @@
     } else
         _editingDelegateShouldInsertText = YES;
 
+    _shouldWriteEmptyData = [[plugInController.parameters valueForKey:@"EditingDelegateShouldWriteEmptyData"] boolValue];
     _shouldOverridePerformTwoStepDrop = [[plugInController.parameters valueForKey:@"BundleOverridePerformTwoStepDrop"] boolValue];
 
     _WKRemoteObjectInterface *interface = [_WKRemoteObjectInterface remoteObjectInterfaceWithProtocol:@protocol(BundleEditingDelegateProtocol)];
@@ -83,7 +85,7 @@
 
 - (NSDictionary<NSString *, NSData *> *)_webProcessPlugInBrowserContextController:(WKWebProcessPlugInBrowserContextController *)controller pasteboardDataForRange:(WKWebProcessPlugInRangeHandle *)range
 {
-    return @{ @"org.webkit.data" : [NSData dataWithBytesNoCopy:(void*)"hello" length:5 freeWhenDone:NO] };
+    return @{ @"org.webkit.data" : _shouldWriteEmptyData ? NSData.data : [NSData dataWithBytesNoCopy:(void*)"hello" length:5 freeWhenDone:NO] };
 }
 
 - (void)_webProcessPlugInBrowserContextControllerDidWriteToPasteboard:(WKWebProcessPlugInBrowserContextController *)controller