From 55c270253b1ae81a4493a05e7c68b272e42f21e5 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 30 Jan 2019 14:57:24 +0100
Subject: [PATCH] worker: throw for duplicates in transfer list

Throw a `DataCloneError` exception when encountering duplicate
`ArrayBuffer`s or `MessagePort`s in the transfer list.

Fixes: https://github.com/nodejs/node/issues/25786

PR-URL: https://github.com/nodejs/node/pull/25815
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 src/node_messaging.cc                         | 19 ++++++++++++
 ...-worker-message-port-transfer-duplicate.js | 29 +++++++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 test/parallel/test-worker-message-port-transfer-duplicate.js

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
index aa4b92bf749eac..6e8e3e8ad76e03 100644
--- a/src/node_messaging.cc
+++ b/src/node_messaging.cc
@@ -289,6 +289,15 @@ Maybe<bool> Message::Serialize(Environment* env,
         // take ownership of its memory, copying the buffer will have to do.
         if (!ab->IsNeuterable() || ab->IsExternal())
           continue;
+        if (std::find(array_buffers.begin(), array_buffers.end(), ab) !=
+            array_buffers.end()) {
+          ThrowDataCloneException(
+              env,
+              FIXED_ONE_BYTE_STRING(
+                  env->isolate(),
+                  "Transfer list contains duplicate ArrayBuffer"));
+          return Nothing<bool>();
+        }
         // We simply use the array index in the `array_buffers` list as the
         // ID that we write into the serialized buffer.
         uint32_t id = array_buffers.size();
@@ -314,6 +323,15 @@ Maybe<bool> Message::Serialize(Environment* env,
                   "MessagePort in transfer list is already detached"));
           return Nothing<bool>();
         }
+        if (std::find(delegate.ports_.begin(), delegate.ports_.end(), port) !=
+            delegate.ports_.end()) {
+          ThrowDataCloneException(
+              env,
+              FIXED_ONE_BYTE_STRING(
+                  env->isolate(),
+                  "Transfer list contains duplicate MessagePort"));
+          return Nothing<bool>();
+        }
         delegate.ports_.push_back(port);
         continue;
       }
@@ -601,6 +619,7 @@ void MessagePort::OnClose() {
 }
 
 std::unique_ptr<MessagePortData> MessagePort::Detach() {
+  CHECK(data_);
   Mutex::ScopedLock lock(data_->mutex_);
   data_->owner_ = nullptr;
   return std::move(data_);
diff --git a/test/parallel/test-worker-message-port-transfer-duplicate.js b/test/parallel/test-worker-message-port-transfer-duplicate.js
new file mode 100644
index 00000000000000..c893556d8d2c48
--- /dev/null
+++ b/test/parallel/test-worker-message-port-transfer-duplicate.js
@@ -0,0 +1,29 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { MessageChannel } = require('worker_threads');
+
+// Test that passing duplicate transferrables in the transfer list throws
+// DataCloneError exceptions.
+
+{
+  const { port1, port2 } = new MessageChannel();
+  port2.once('message', common.mustNotCall());
+
+  const port3 = new MessageChannel().port1;
+  assert.throws(() => {
+    port1.postMessage(port3, [port3, port3]);
+  }, /^DataCloneError: Transfer list contains duplicate MessagePort$/);
+  port1.close();
+}
+
+{
+  const { port1, port2 } = new MessageChannel();
+  port2.once('message', common.mustNotCall());
+
+  const buf = new Uint8Array(10);
+  assert.throws(() => {
+    port1.postMessage(buf, [buf.buffer, buf.buffer]);
+  }, /^DataCloneError: Transfer list contains duplicate ArrayBuffer$/);
+  port1.close();
+}