buffer: disallow ArrayBuffer transfer on pooled buffer

Author: legendecas

deps/v8/src/api/api.cc

@@ -8829,6 +8829,7 @@ void v8::ArrayBuffer::SetDetachKey(v8::Local<v8::Value> key) {
   auto obj = Utils::OpenDirectHandle(this);
   auto i_key = Utils::OpenDirectHandle(*key);
   obj->set_detach_key(*i_key);
+  obj->set_is_detachable(false);
 }
 
 size_t v8::ArrayBuffer::ByteLength() const {

lib/buffer.js

@@ -129,6 +129,13 @@ const {
   createUnsafeBuffer,
 } = require('internal/buffer');
 
+const {
+  namespace: {
+    addDeserializeCallback,
+    isBuildingSnapshot,
+  },
+} = require('internal/v8/startup_snapshot');
+
 FastBuffer.prototype.constructor = Buffer;
 Buffer.prototype = FastBuffer.prototype;
 addBufferPrototypeMethods(Buffer.prototype);
@@ -159,6 +166,13 @@ function createPool() {
   poolOffset = 0;
 }
 createPool();
+if (isBuildingSnapshot()) {
+  addDeserializeCallback(() => {
+    // TODO(legendecas): ArrayBuffer.[[ArrayBufferDetachKey]] is not been serialized.
+    // Remove this callback when snapshot serialization supports it.
+    createPool();
+  });
+}
 
 function alignPool() {
   // Ensure aligned slices

lib/internal/buffer.js

@@ -31,6 +31,7 @@ const {
   ucs2Write,
   utf8WriteStatic,
   getZeroFillToggle,
+  setDetachKey,
 } = internalBinding('buffer');
 
 const {
@@ -45,6 +46,7 @@ const {
   },
   addAfterUserSerializeCallback,
 } = require('internal/v8/startup_snapshot');
+const { isArrayBuffer } = require('util/types');
 
 // Temporary buffers to convert numbers.
 const float32Array = new Float32Array(1);
@@ -1075,6 +1077,10 @@ function markAsUntransferable(obj) {
   if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null)
     return;  // This object is a primitive and therefore already untransferable.
   obj[untransferable_object_private_symbol] = true;
+
+  if (isArrayBuffer(obj)) {
+    setDetachKey(obj, Symbol('unique_detach_key_for_untransferable_arraybuffer'));
+  }
 }
 
 // This simply checks if the object is marked as untransferable and doesn't

src/node_buffer.cc

@@ -1394,6 +1394,15 @@ static void Atob(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(error_code);
 }
 
+static void SetDetachKey(const FunctionCallbackInfo<Value>& args) {
+  CHECK_EQ(args.Length(), 2);
+  CHECK(args[0]->IsArrayBuffer());
+
+  Local<ArrayBuffer> ab = args[0].As<ArrayBuffer>();
+  Local<Value> key = args[1];
+  ab->SetDetachKey(key);
+}
+
 namespace {
 
 std::pair<void*, size_t> DecomposeBufferToParts(Local<Value> buffer) {
@@ -1627,6 +1636,7 @@ void Initialize(Local<Object> target,
                 &fast_write_string_utf8);
 
   SetMethod(context, target, "getZeroFillToggle", GetZeroFillToggle);
+  SetMethod(context, target, "setDetachKey", SetDetachKey);
 }
 
 }  // anonymous namespace
@@ -1681,6 +1691,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
 
   registry->Register(Atob);
   registry->Register(Btoa);
+
+  registry->Register(SetDetachKey);
 }
 
 }  // namespace Buffer

test/parallel/test-buffer-pool-untransferable.js

@@ -21,3 +21,9 @@ assert.throws(() => port1.postMessage(a, [ a.buffer ]), {
 // Verify that the pool ArrayBuffer has not actually been transferred:
 assert.strictEqual(a.buffer, b.buffer);
 assert.strictEqual(a.length, length);
+
+// Verify that ArrayBuffer.prototype.transfer() also throws.
+assert.throws(() => a.buffer.transfer(), {
+  name: 'TypeError',
+});
+assert.strictEqual(a.buffer, b.buffer);