ffi: reject detached ArrayBuffers as pointers

lib/internal/ffi/fast-api.js

@@ -1,13 +1,16 @@
 'use strict';
 
 const {
+  ArrayBufferPrototypeGetDetached,
   ArrayPrototypeIncludes,
+  DataViewPrototypeGetBuffer,
   NumberIsInteger,
   ObjectDefineProperty,
   ReflectApply,
   SafeWeakMap,
   StringPrototypeIncludes,
   TypeError,
+  TypedArrayPrototypeGetBuffer,
 } = primordials;
 
 const {
@@ -16,7 +19,9 @@ const {
 
 const {
   isAnyArrayBuffer,
+  isArrayBuffer,
   isArrayBufferView,
+  isDataView,
 } = require('internal/util/types');
 
 const {
@@ -158,6 +163,28 @@ function getStringConversionPointer(state, value, index) {
   return entry.pointer;
 }
 
+function getRawPointerArg(value, index) {
+  let buffer;
+  let isView = false;
+  if (isArrayBuffer(value)) {
+    buffer = value;
+  } else if (isArrayBufferView(value)) {
+    isView = true;
+    buffer = isDataView(value) ?
+      DataViewPrototypeGetBuffer(value) :
+      TypedArrayPrototypeGetBuffer(value);
+  }
+
+  if (buffer !== undefined && isArrayBuffer(buffer) &&
+      ArrayBufferPrototypeGetDetached(buffer)) {
+    throwFFIArgError(isView ?
+      `Argument ${index} is an ArrayBufferView backed by a detached ArrayBuffer` :
+      `Argument ${index} is a detached ArrayBuffer`);
+  }
+
+  return getRawPointer(value);
+}
+
 function convertPointerArg(type, value, stringState, index) {
   if (needsNullPointerConversion(type) &&
       (value === null || value === undefined)) {
@@ -167,7 +194,7 @@ function convertPointerArg(type, value, stringState, index) {
     return getStringConversionPointer(stringState, value, index);
   }
   if (hasPointerMemoryArg(type, value)) {
-    return getRawPointer(value);
+    return getRawPointerArg(value, index);
   }
   // Pointer-like values (e.g. BigInt addresses) are passed through, matching
   // ToFFIArgument in src/ffi/types.cc and the single-argument fast path.
@@ -276,7 +303,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
         if (fastBufferInvoke !== undefined) {
           return fastBufferInvoke(arg);
         }
-        arg = getRawPointer(arg);
+        arg = getRawPointerArg(arg, 0);
       }
       return rawFn(arg);
     };

src/ffi/data.cc

@@ -685,7 +685,7 @@ void ExportBytes(const FunctionCallbackInfo<Value>& args) {
       args[0]->IsArrayBufferView()) {
     view.ReadValue(args[0]);
     if (view.WasDetached()) {
-      THROW_ERR_INVALID_ARG_VALUE(env, "Invalid ArrayBufferView backing store");
+      THROW_ERR_INVALID_ARG_VALUE(env, "ArrayBuffer is detached");
       return;
     }
   } else {
@@ -749,15 +749,26 @@ void GetRawPointer(const FunctionCallbackInfo<Value>& args) {
   std::shared_ptr<BackingStore> store;
 
   if (args[0]->IsArrayBuffer()) {
-    store = args[0].As<ArrayBuffer>()->GetBackingStore();
+    Local<ArrayBuffer> buffer = args[0].As<ArrayBuffer>();
+    if (buffer->WasDetached()) {
+      THROW_ERR_INVALID_ARG_VALUE(env, "ArrayBuffer is detached");
+      return;
+    }
+    store = buffer->GetBackingStore();
   } else if (args[0]->IsSharedArrayBuffer()) {
     store = args[0].As<SharedArrayBuffer>()->GetBackingStore();
   } else if (args[0]->IsArrayBufferView()) {
+    Local<ArrayBufferView> view = args[0].As<ArrayBufferView>();
+    if (view->Buffer()->WasDetached()) {
+      THROW_ERR_INVALID_ARG_VALUE(
+          env, "ArrayBufferView is backed by a detached ArrayBuffer");
+      return;
+    }
     // Access the store here to ensure that it exists. Small typed arrays
     // may not have a store until this point and can instead be stored
     // entirely in-heap.
-    store = args[0].As<ArrayBufferView>()->Buffer()->GetBackingStore();
-    offset = args[0].As<ArrayBufferView>()->ByteOffset();
+    store = view->Buffer()->GetBackingStore();
+    offset = view->ByteOffset();
   } else {
     THROW_ERR_INVALID_ARG_TYPE(
         env,

src/ffi/fast.cc

@@ -246,18 +246,45 @@ extern "C" uintptr_t node_ffi_fast_buffer_data(v8::Local<v8::Value> value,
   // returns zero after throwing, preventing the native target from seeing an
   // invalid pointer value.
   constexpr uintptr_t kInvalidBuffer = std::numeric_limits<uintptr_t>::max();
+  v8::Isolate* isolate = options != nullptr ? options->isolate : nullptr;
 
   // Accept only memory-backed JS values in the native helper. Other pointer
   // conversions, including strings, stay in the JS wrapper so their temporary
   // lifetime is explicit.
   if (value->IsArrayBufferView()) {
+    v8::Local<v8::ArrayBufferView> view = value.As<v8::ArrayBufferView>();
+    if (view->Buffer()->WasDetached()) {
+      if (isolate != nullptr) {
+        // No HandleScope is active during a Fast API call, so open one before
+        // creating the error object.
+        v8::HandleScope scope(isolate);
+        THROW_ERR_INVALID_ARG_VALUE(
+            isolate,
+            "Argument %u is an ArrayBufferView backed by a detached "
+            "ArrayBuffer",
+            index);
+      }
+      return kInvalidBuffer;
+    }
     return PointerFromValue(value);
   }
-  if (value->IsArrayBuffer() || value->IsSharedArrayBuffer()) {
+  if (value->IsArrayBuffer()) {
+    if (value.As<v8::ArrayBuffer>()->WasDetached()) {
+      if (isolate != nullptr) {
+        // No HandleScope is active during a Fast API call, so open one before
+        // creating the error object.
+        v8::HandleScope scope(isolate);
+        THROW_ERR_INVALID_ARG_VALUE(
+            isolate, "Argument %u is a detached ArrayBuffer", index);
+      }
+      return kInvalidBuffer;
+    }
+    return PointerFromValue(value);
+  }
+  if (value->IsSharedArrayBuffer()) {
     return PointerFromValue(value);
   }
 
-  v8::Isolate* isolate = options != nullptr ? options->isolate : nullptr;
   if (isolate != nullptr) {
     // No HandleScope is active during a Fast API call, so open one before
     // creating the error object.

src/ffi/types.cc

@@ -700,6 +700,14 @@ Maybe<FFIArgumentCategory> ToFFIArgument(Environment* env,
       // invalidating that backing store during the active FFI call is
       // unsupported and dangerous.
       Local<ArrayBufferView> view = arg.As<ArrayBufferView>();
+      if (view->Buffer()->WasDetached()) {
+        THROW_ERR_INVALID_ARG_VALUE(
+            env,
+            "Argument %u is an ArrayBufferView backed by a detached "
+            "ArrayBuffer",
+            index);
+        return {};
+      }
       std::shared_ptr<BackingStore> store = view->Buffer()->GetBackingStore();
 
       if (!store) {
@@ -721,6 +729,11 @@ Maybe<FFIArgumentCategory> ToFFIArgument(Environment* env,
       // that backing store during the active FFI call is unsupported and
       // dangerous.
       Local<ArrayBuffer> buffer = arg.As<ArrayBuffer>();
+      if (buffer->WasDetached()) {
+        THROW_ERR_INVALID_ARG_VALUE(
+            env, "Argument %u is a detached ArrayBuffer", index);
+        return {};
+      }
       std::shared_ptr<BackingStore> store = buffer->GetBackingStore();
 
       if (!store) {

test/ffi/test-ffi-fast-buffer.js

@@ -98,6 +98,10 @@ test('fast FFI string buffers survive reentrant callbacks', {
 
 test('optimized buffer signatures preserve pointer-like conversions', () => {
   const lib = new ffi.DynamicLibrary(libraryPath);
+  const asPointer = lib.getFunction('pointer_to_usize', {
+    arguments: ['pointer'],
+    return: 'u64',
+  });
   const asBuffer = lib.getFunction('pointer_to_usize', {
     arguments: ['buffer'],
     return: 'u64',
@@ -107,6 +111,10 @@ test('optimized buffer signatures preserve pointer-like conversions', () => {
     return: 'u64',
   });
 
+  function callPointer(value) {
+    return asPointer(value);
+  }
+
   function callBuffer(value) {
     return asBuffer(value);
   }
@@ -117,17 +125,34 @@ test('optimized buffer signatures preserve pointer-like conversions', () => {
 
   try {
     for (let i = 0; i < 100_000; i++) {
+      assert.strictEqual(callPointer(0n), 0n);
       assert.strictEqual(callBuffer(0n), 0n);
       assert.strictEqual(callArrayBuffer(0n), 0n);
     }
 
-    for (const call of [callBuffer, callArrayBuffer]) {
+    for (const call of [callPointer, callBuffer, callArrayBuffer]) {
       assert.strictEqual(call(null), 0n);
       assert.strictEqual(call(undefined), 0n);
       assert.notStrictEqual(call('ffi'), 0n);
 
       const bytes = Buffer.alloc(1);
       assert.strictEqual(call(bytes), ffi.getRawPointer(bytes));
+
+      const arrayBuffer = new ArrayBuffer(8);
+      const typedArray = new Uint8Array(arrayBuffer);
+      const dataView = new DataView(arrayBuffer);
+      arrayBuffer.transfer();
+
+      assert.throws(() => call(arrayBuffer), {
+        code: 'ERR_INVALID_ARG_VALUE',
+        message: 'Argument 0 is a detached ArrayBuffer',
+      });
+      for (const view of [typedArray, dataView]) {
+        assert.throws(() => call(view), {
+          code: 'ERR_INVALID_ARG_VALUE',
+          message: 'Argument 0 is an ArrayBufferView backed by a detached ArrayBuffer',
+        });
+      }
     }
   } finally {
     lib.close();

test/ffi/test-ffi-memory.js

@@ -146,6 +146,41 @@ test('ffi getRawPointer returns raw addresses for byte sources', () => {
   assert.strictEqual(sharedViewPointer, sharedArrayBufferPointer + 2n);
 });
 
+test('ffi rejects detached array buffers and views as pointers', () => {
+  const arrayBuffer = new ArrayBuffer(8);
+  const typedArray = new Uint8Array(arrayBuffer);
+  const dataView = new DataView(arrayBuffer);
+
+  arrayBuffer.transfer();
+
+  assert.throws(() => ffi.exportArrayBuffer(arrayBuffer, 0n, 0), {
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: 'ArrayBuffer is detached',
+  });
+
+  for (const [value, rawPointerMessage, argumentMessage] of [
+    [
+      arrayBuffer,
+      'ArrayBuffer is detached',
+      'Argument 0 is a detached ArrayBuffer',
+    ],
+    ...[typedArray, dataView].map((view) => [
+      view,
+      'ArrayBufferView is backed by a detached ArrayBuffer',
+      'Argument 0 is an ArrayBufferView backed by a detached ArrayBuffer',
+    ]),
+  ]) {
+    assert.throws(() => ffi.getRawPointer(value), {
+      code: 'ERR_INVALID_ARG_VALUE',
+      message: rawPointerMessage,
+    });
+    assert.throws(() => symbols.pointer_to_usize(value), {
+      code: 'ERR_INVALID_ARG_VALUE',
+      message: argumentMessage,
+    });
+  }
+});
+
 test('ffi exportString and exportBuffer copy data into native memory', () => {
   withAllocations(common.mustCall((alloc) => {
     const stringPtr = alloc(16);