fix: postMessage crash with invalid transferrable (#46639)

shell/browser/api/electron_api_utility_process.cc

@@ -330,6 +330,9 @@ void UtilityProcessWrapper::PostMessage(gin::Arguments* args) {
     return;
 
   blink::TransferableMessage transferable_message;
+  gin_helper::ErrorThrower thrower(args->isolate());
+
+  // |message| is any value that can be serialized to StructuredClone.
   v8::Local<v8::Value> message_value;
   if (args->GetNext(&message_value)) {
     if (!electron::SerializeV8Value(args->isolate(), message_value,
@@ -342,9 +345,25 @@ void UtilityProcessWrapper::PostMessage(gin::Arguments* args) {
   v8::Local<v8::Value> transferables;
   std::vector<gin::Handle<MessagePort>> wrapped_ports;
   if (args->GetNext(&transferables)) {
+    std::vector<v8::Local<v8::Value>> wrapped_port_values;
+    if (!gin::ConvertFromV8(args->isolate(), transferables,
+                            &wrapped_port_values)) {
+      thrower.ThrowTypeError("transferables must be an array of MessagePorts");
+      return;
+    }
+
+    for (size_t i = 0; i < wrapped_port_values.size(); ++i) {
+      if (!gin_helper::IsValidWrappable(wrapped_port_values[i],
+                                        &MessagePort::kWrapperInfo)) {
+        thrower.ThrowTypeError(
+            base::StrCat({"Port at index ", base::NumberToString(i),
+                          " is not a valid port"}));
+        return;
+      }
+    }
+
     if (!gin::ConvertFromV8(args->isolate(), transferables, &wrapped_ports)) {
-      gin_helper::ErrorThrower(args->isolate())
+      thrower.ThrowTypeError("Passed an invalid MessagePort");
-          .ThrowTypeError("Invalid value for transfer");
       return;
     }
   }

shell/browser/api/message_port.cc

@@ -17,6 +17,7 @@
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/error_thrower.h"
 #include "shell/common/gin_helper/event_emitter_caller.h"
+#include "shell/common/gin_helper/wrappable.h"
 #include "shell/common/node_includes.h"
 #include "shell/common/v8_util.h"
 #include "third_party/abseil-cpp/absl/container/flat_hash_set.h"
@@ -26,25 +27,6 @@
 
 namespace electron {
 
-namespace {
-
-bool IsValidWrappable(const v8::Local<v8::Value>& val) {
-  if (!val->IsObject())
-    return false;
-
-  v8::Local<v8::Object> port = val.As<v8::Object>();
-
-  if (port->InternalFieldCount() != gin::kNumberOfInternalFields)
-    return false;
-
-  const auto* info = static_cast<gin::WrapperInfo*>(
-      port->GetAlignedPointerFromInternalField(gin::kWrapperInfoIndex));
-
-  return info && info->embedder == gin::kEmbedderNativeGin;
-}
-
-}  // namespace
-
 gin::WrapperInfo MessagePort::kWrapperInfo = {gin::kEmbedderNativeGin};
 
 MessagePort::MessagePort() = default;
@@ -77,17 +59,15 @@ void MessagePort::PostMessage(gin::Arguments* args) {
   blink::TransferableMessage transferable_message;
   gin_helper::ErrorThrower thrower(args->isolate());
 
+  // |message| is any value that can be serialized to StructuredClone.
   v8::Local<v8::Value> message_value;
-  if (!args->GetNext(&message_value)) {
+  if (args->GetNext(&message_value)) {
-    thrower.ThrowTypeError("Expected at least one argument to postMessage");
-    return;
-  }
-
     if (!electron::SerializeV8Value(args->isolate(), message_value,
                                     &transferable_message)) {
       // SerializeV8Value sets an exception.
       return;
     }
+  }
 
   v8::Local<v8::Value> transferables;
   std::vector<gin::Handle<MessagePort>> wrapped_ports;
@@ -100,7 +80,8 @@ void MessagePort::PostMessage(gin::Arguments* args) {
     }
 
     for (unsigned i = 0; i < wrapped_port_values.size(); ++i) {
-      if (!IsValidWrappable(wrapped_port_values[i])) {
+      if (!gin_helper::IsValidWrappable(wrapped_port_values[i],
+                                        &MessagePort::kWrapperInfo)) {
         thrower.ThrowTypeError("Port at index " + base::NumberToString(i) +
                                " is not a valid port");
         return;

shell/common/gin_helper/wrappable.cc

@@ -10,6 +10,23 @@
 
 namespace gin_helper {
 
+bool IsValidWrappable(const v8::Local<v8::Value>& val,
+                      const gin::WrapperInfo* wrapper_info) {
+  if (!val->IsObject())
+    return false;
+
+  v8::Local<v8::Object> port = val.As<v8::Object>();
+  if (port->InternalFieldCount() != gin::kNumberOfInternalFields)
+    return false;
+
+  const gin::WrapperInfo* info = static_cast<gin::WrapperInfo*>(
+      port->GetAlignedPointerFromInternalField(gin::kWrapperInfoIndex));
+  if (info != wrapper_info)
+    return false;
+
+  return true;
+}
+
 WrappableBase::WrappableBase() = default;
 
 WrappableBase::~WrappableBase() {

shell/common/gin_helper/wrappable.h

@@ -11,6 +11,9 @@
 
 namespace gin_helper {
 
+bool IsValidWrappable(const v8::Local<v8::Value>& obj,
+                      const gin::WrapperInfo* wrapper_info);
+
 namespace internal {
 
 void* FromV8Impl(v8::Isolate* isolate, v8::Local<v8::Value> val);

spec/api-ipc-spec.ts

@@ -236,6 +236,23 @@ describe('ipc module', () => {
       expect(ev.senderFrame.routingId).to.equal(w.webContents.mainFrame.routingId);
     });
 
+    it('throws when the transferable is invalid', async () => {
+      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, contextIsolation: false } });
+      w.loadURL('about:blank');
+      const p = once(ipcMain, 'port');
+      await w.webContents.executeJavaScript(`(${function () {
+        try {
+          const buffer = new ArrayBuffer(10);
+          // @ts-expect-error
+          require('electron').ipcRenderer.postMessage('port', '', [buffer]);
+        } catch (e) {
+          require('electron').ipcRenderer.postMessage('port', { error: (e as Error).message });
+        }
+      }})()`);
+      const [, msg] = await p;
+      expect(msg.error).to.eql('Invalid value for transfer');
+    });
+
     it('can communicate between main and renderer', async () => {
       const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, contextIsolation: false } });
       w.loadURL('about:blank');
@@ -411,6 +428,20 @@ describe('ipc module', () => {
         expect(port2).not.to.be.null();
       });
 
+      it('should not throw when supported values are passed as message', () => {
+        const { port1 } = new MessageChannelMain();
+
+        // @ts-expect-error - this shouldn't crash.
+        expect(() => { port1.postMessage(); }).to.not.throw();
+
+        expect(() => { port1.postMessage(undefined); }).to.not.throw();
+        expect(() => { port1.postMessage(42); }).to.not.throw();
+        expect(() => { port1.postMessage(false); }).to.not.throw();
+        expect(() => { port1.postMessage([]); }).to.not.throw();
+        expect(() => { port1.postMessage('hello'); }).to.not.throw();
+        expect(() => { port1.postMessage({ hello: 'goodbye' }); }).to.not.throw();
+      });
+
       it('throws an error when an invalid parameter is sent to postMessage', () => {
         const { port1 } = new MessageChannelMain();