Skip to content

Commit

Permalink
fix: crash in MessagePortMain with some postMessage params (#37725)
Browse files Browse the repository at this point in the history
* fix: crash in MessagePortMain postMessage

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* Update shell/browser/api/message_port.cc

Co-authored-by: Charles Kerr <charles@charleskerr.com>

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
  • Loading branch information
trop[bot] and codebytere committed Mar 28, 2023
1 parent 4a7bf76 commit 352ab57
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 6 deletions.
44 changes: 39 additions & 5 deletions shell/browser/api/message_port.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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;
Expand All @@ -47,10 +66,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;
}

Expand All @@ -60,18 +80,32 @@ 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;
}
}

// 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;
}
}
Expand Down
27 changes: 26 additions & 1 deletion spec/api-ipc-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 352ab57

Please sign in to comment.