fix: utilityProcess.postMessage crash with invalid transferrable

electron · Mar 4, 2024 · b737cdc · b737cdc
1 parent d5912fd
commit b737cdc
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 22 deletions.
diff --git a/shell/browser/api/electron_api_utility_process.cc b/shell/browser/api/electron_api_utility_process.cc
@@ -53,6 +53,25 @@ GetAllUtilityProcessWrappers() {
   return *s_all_utility_process_wrappers;
 }
 
+namespace {
+
+bool IsValidWrappable(const v8::Local<v8::Value>& obj) {
+  v8::Local<v8::Object> port = v8::Local<v8::Object>::Cast(obj);
+
+  if (!port->IsObject())
+    return false;
+
+  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
+
 namespace api {
 
 gin::WrapperInfo UtilityProcessWrapper::kWrapperInfo = {
@@ -265,28 +284,46 @@ void UtilityProcessWrapper::PostMessage(gin::Arguments* args) {
     return;
 
   blink::TransferableMessage transferable_message;
+
+  auto* isolate = args->isolate();
+  gin_helper::ErrorThrower thrower(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,
-                                    &transferable_message)) {
-      // SerializeV8Value sets an exception.
-      return;
-    }
+  args->GetNext(&message_value);
+
+  if (!electron::SerializeV8Value(isolate, message_value,
+                                  &transferable_message)) {
+    // SerializeV8Value sets an exception.
+    return;
   }
 
   v8::Local<v8::Value> transferables;
   std::vector<gin::Handle<MessagePort>> wrapped_ports;
   if (args->GetNext(&transferables)) {
-    if (!gin::ConvertFromV8(args->isolate(), transferables, &wrapped_ports)) {
-      gin_helper::ErrorThrower(args->isolate())
-          .ThrowTypeError("Invalid value for transfer");
+    std::vector<v8::Local<v8::Value>> wrapped_port_values;
+    if (!gin::ConvertFromV8(isolate, transferables, &wrapped_port_values)) {
+      thrower.ThrowTypeError("transferables must be an array of MessagePorts");
+      return;
+    }
+
+    for (unsigned i = 0; i < wrapped_port_values.size(); ++i) {
+      if (!IsValidWrappable(wrapped_port_values[i])) {
+        thrower.ThrowTypeError("Port at index " + base::NumberToString(i) +
+                               " is not a valid port");
+        return;
+      }
+    }
+
+    if (!gin::ConvertFromV8(isolate, transferables, &wrapped_ports)) {
+      thrower.ThrowTypeError("Passed an invalid MessagePort");
       return;
     }
   }
 
   bool threw_exception = false;
-  transferable_message.ports = MessagePort::DisentanglePorts(
-      args->isolate(), wrapped_ports, &threw_exception);
+  transferable_message.ports =
+      MessagePort::DisentanglePorts(isolate, wrapped_ports, &threw_exception);
   if (threw_exception)
     return;
 

diff --git a/shell/browser/api/message_port.cc b/shell/browser/api/message_port.cc
@@ -65,26 +65,29 @@ gin::Handle<MessagePort> MessagePort::Create(v8::Isolate* isolate) {
 void MessagePort::PostMessage(gin::Arguments* args) {
   if (!IsEntangled())
     return;
+
   DCHECK(!IsNeutered());
 
   blink::TransferableMessage transferable_message;
-  gin_helper::ErrorThrower thrower(args->isolate());
 
+  auto* isolate = args->isolate();
+  gin_helper::ErrorThrower thrower(isolate);
+
+  // |message| is any value that can be serialized to StructuredClone.
   v8::Local<v8::Value> message_value;
-  if (!args->GetNext(&message_value)) {
-    thrower.ThrowTypeError("Expected at least one argument to postMessage");
+  args->GetNext(&message_value);
+
+  if (!electron::SerializeV8Value(isolate, message_value,
+                                  &transferable_message)) {
+    // SerializeV8Value sets an exception.
     return;
   }
 
-  electron::SerializeV8Value(args->isolate(), message_value,
-                             &transferable_message);
-
   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)) {
+    if (!gin::ConvertFromV8(isolate, transferables, &wrapped_port_values)) {
       thrower.ThrowTypeError("transferables must be an array of MessagePorts");
       return;
     }
@@ -97,7 +100,7 @@ void MessagePort::PostMessage(gin::Arguments* args) {
       }
     }
 
-    if (!gin::ConvertFromV8(args->isolate(), transferables, &wrapped_ports)) {
+    if (!gin::ConvertFromV8(isolate, transferables, &wrapped_ports)) {
       thrower.ThrowTypeError("Passed an invalid MessagePort");
       return;
     }
@@ -113,8 +116,8 @@ void MessagePort::PostMessage(gin::Arguments* args) {
   }
 
   bool threw_exception = false;
-  transferable_message.ports = MessagePort::DisentanglePorts(
-      args->isolate(), wrapped_ports, &threw_exception);
+  transferable_message.ports =
+      MessagePort::DisentanglePorts(isolate, wrapped_ports, &threw_exception);
   if (threw_exception)
     return;
 

diff --git a/spec/api-ipc-spec.ts b/spec/api-ipc-spec.ts
@@ -233,6 +233,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');

diff --git a/spec/api-utility-process-spec.ts b/spec/api-utility-process-spec.ts
@@ -266,6 +266,20 @@ describe('utilityProcess module', () => {
       await exit;
     });
 
+    it('throws when an invalid transferrable is passed', () => {
+      const child = utilityProcess.fork(path.join(fixturesPath, 'post-message.js'));
+      expect(() => {
+        // @ts-expect-error
+        const buffer = new ArrayBuffer();
+        child.postMessage('Hello', [buffer as any]);
+      }).to.throw(/Port at index 0 is not a valid port/);
+
+      expect(() => {
+        // @ts-expect-error
+        child.postMessage('hey', [false]);
+      }).to.throw(/Port at index 0 is not a valid port/);
+    });
+
     it('supports queuing messages on the receiving end', async () => {
       const child = utilityProcess.fork(path.join(fixturesPath, 'post-message-queue.js'));
       const p = once(child, 'spawn');