Changeset 259346 in webkit

Timestamp:

Apr 1, 2020, 9:01:36 AM (5 years ago)

Author:

Wenson Hsieh

Message:

Make WebPasteboardProxy::didModifyContentsOfPasteboard robust when pasteboardName is null
​https://bugs.webkit.org/show_bug.cgi?id=209848
<rdar://problem/61121810>

Reviewed by Megan Gardner and David Kilzer.

Add more IPC message checks in WebPasteboardProxy; see below for more detail.

• UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:

Rename what is currently MESSAGE_CHECK to MESSAGE_CHECK_COMPLETION, and introduce two more message check macros:
MESSAGE_CHECK_WITH_RETURN_VALUE, which supports a return value, and MESSAGE_CHECK, which returns with no value.

(WebKit::WebPasteboardProxy::canAccessPasteboardData const):

Replace the early returns when pasteboardName is empty or when the web process for the given connection is null
with MESSAGE_CHECKs. When the web content process is well-behaved, these early returns should never be hit.

(WebKit::WebPasteboardProxy::didModifyContentsOfPasteboard):

Similarly, replace this early return with a message check, and additionally MESSAGE_CHECK when the pasteboard
name is empty. This addresses the main issue caught by this radar.

(WebKit::WebPasteboardProxy::setPasteboardBufferForType):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/Cocoa/WebPasteboardProxyCocoa.mm (modified) (4 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-04-01  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Make WebPasteboardProxy::didModifyContentsOfPasteboard robust when pasteboardName is null
+        https://bugs.webkit.org/show_bug.cgi?id=209848
+        <rdar://problem/61121810>
+
+        Reviewed by Megan Gardner and David Kilzer.
+
+        Add more IPC message checks in WebPasteboardProxy; see below for more detail.
+
+        * UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:
+
+        Rename what is currently MESSAGE_CHECK to MESSAGE_CHECK_COMPLETION, and introduce two more message check macros:
+        MESSAGE_CHECK_WITH_RETURN_VALUE, which supports a return value, and MESSAGE_CHECK, which returns with no value.
+
+        (WebKit::WebPasteboardProxy::canAccessPasteboardData const):
+
+        Replace the early returns when pasteboardName is empty or when the web process for the given connection is null
+        with `MESSAGE_CHECK`s. When the web content process is well-behaved, these early returns should never be hit.
+
+        (WebKit::WebPasteboardProxy::didModifyContentsOfPasteboard):
+
+        Similarly, replace this early return with a message check, and additionally `MESSAGE_CHECK` when the pasteboard
+        name is empty. This addresses the main issue caught by this radar.
+
+        (WebKit::WebPasteboardProxy::setPasteboardBufferForType):
+
 2020-04-01  Victor M. Jaquez <vjaquez@igalia.com>
 

trunk/Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm

@@ -39 +39 @@
 #import <wtf/URL.h>
 
-#define MESSAGE_CHECK(assertion, completion) MESSAGE_CHECK_COMPLETION_BASE(assertion, (&connection), completion)
+#define MESSAGE_CHECK(assertion) MESSAGE_CHECK_BASE(assertion, (&connection))
+#define MESSAGE_CHECK_WITH_RETURN_VALUE(assertion, returnValue) MESSAGE_CHECK_WITH_RETURN_VALUE_BASE(assertion, (&connection), returnValue)
+#define MESSAGE_CHECK_COMPLETION(assertion, completion) MESSAGE_CHECK_COMPLETION_BASE(assertion, (&connection), completion)
 
 namespace WebKit {
@@ -74 +76 @@
 bool WebPasteboardProxy::canAccessPasteboardData(IPC::Connection& connection, const String& pasteboardName) const
 {
-    if (pasteboardName.isEmpty()) {
-        ASSERT_NOT_REACHED();
-        return false;
-    }
+    MESSAGE_CHECK_WITH_RETURN_VALUE(!pasteboardName.isEmpty(), false);
 
     auto* process = webProcessProxyForConnection(connection);
-    if (!process)
-        return false;
+    MESSAGE_CHECK_WITH_RETURN_VALUE(process, false);
 
     for (auto* page : process->pages()) {
@@ -107 +105 @@
 void WebPasteboardProxy::didModifyContentsOfPasteboard(IPC::Connection& connection, const String& pasteboardName, int64_t previousChangeCount, int64_t newChangeCount)
 {
+    MESSAGE_CHECK(!pasteboardName.isEmpty());
+
     auto* process = webProcessProxyForConnection(connection);
-    if (!process)
-        return;
+    MESSAGE_CHECK(process);
 
     auto changeCountAndProcesses = m_pasteboardNameToChangeCountAndProcessesMap.find(pasteboardName);
@@ -301 +300 @@
 
     // SharedMemory::Handle::size() is rounded up to the nearest page.
-    MESSAGE_CHECK(size <= handle.size(), completionHandler(0));
+    MESSAGE_CHECK_COMPLETION(size <= handle.size(), completionHandler(0));
 
     RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::map(handle, SharedMemory::Protection::ReadOnly);