Browse Source

fix: crash in `MessagePortMain` with some `postMessage` params (#37726)

* fix: crash in MessagePortMain postMessage

Co-authored-by: Shelley Vohr <[email protected]>

* Update shell/browser/api/message_port.cc

Co-authored-by: Charles Kerr <[email protected]>

Co-authored-by: Shelley Vohr <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
trop[bot] 2 years ago
parent
commit
f38c3d91a2
2 changed files with 65 additions and 6 deletions
  1. 39 5
      shell/browser/api/message_port.cc
  2. 26 1
      spec/api-ipc-spec.ts

+ 39 - 5
shell/browser/api/message_port.cc

@@ -26,6 +26,25 @@
 
 namespace electron {
 
+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
+
 gin::WrapperInfo MessagePort::kWrapperInfo = {gin::kEmbedderNativeGin};
 
 MessagePort::MessagePort() = default;
@@ -48,10 +67,11 @@ void MessagePort::PostMessage(gin::Arguments* args) {
   DCHECK(!IsNeutered());
 
   blink::TransferableMessage transferable_message;
+  gin_helper::ErrorThrower thrower(args->isolate());
 
   v8::Local<v8::Value> message_value;
   if (!args->GetNext(&message_value)) {
-    args->ThrowTypeError("Expected at least one argument to postMessage");
+    thrower.ThrowTypeError("Expected at least one argument to postMessage");
     return;
   }
 
@@ -61,8 +81,23 @@ void MessagePort::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 (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(args->isolate(), transferables, &wrapped_ports)) {
-      args->ThrowError();
+      thrower.ThrowTypeError("Passed an invalid MessagePort");
       return;
     }
   }
@@ -70,9 +105,8 @@ void MessagePort::PostMessage(gin::Arguments* args) {
   // Make sure we aren't connected to any of the passed-in ports.
   for (unsigned i = 0; i < wrapped_ports.size(); ++i) {
     if (wrapped_ports[i].get() == this) {
-      gin_helper::ErrorThrower(args->isolate())
-          .ThrowError("Port at index " + base::NumberToString(i) +
-                      " contains the source port.");
+      thrower.ThrowError("Port at index " + base::NumberToString(i) +
+                         " contains the source port.");
       return;
     }
   }

+ 26 - 1
spec/api-ipc-spec.ts

@@ -357,6 +357,31 @@ describe('ipc module', () => {
         expect(port2).not.to.be.null();
       });
 
+      it('throws an error when an invalid parameter is sent to postMessage', () => {
+        const { port1 } = new MessageChannelMain();
+
+        expect(() => {
+          const buffer = new ArrayBuffer(10) as any;
+          port1.postMessage(null, [buffer]);
+        }).to.throw(/Port at index 0 is not a valid port/);
+
+        expect(() => {
+          port1.postMessage(null, ['1' as any]);
+        }).to.throw(/Port at index 0 is not a valid port/);
+
+        expect(() => {
+          port1.postMessage(null, [new Date() as any]);
+        }).to.throw(/Port at index 0 is not a valid port/);
+      });
+
+      it('throws when postMessage transferables contains the source port', () => {
+        const { port1 } = new MessageChannelMain();
+
+        expect(() => {
+          port1.postMessage(null, [port1]);
+        }).to.throw(/Port at index 0 contains the source port./);
+      });
+
       it('can send messages within the process', async () => {
         const { port1, port2 } = new MessageChannelMain();
         port2.postMessage('hello');
@@ -425,7 +450,7 @@ describe('ipc module', () => {
         const { port1 } = new MessageChannelMain();
         expect(() => {
           port1.postMessage(null, [null] as any);
-        }).to.throw(/conversion failure/);
+        }).to.throw(/Port at index 0 is not a valid port/);
       });
 
       it('throws when passing duplicate ports', () => {