diff --git a/lib/internal/ffi/fast-api.js b/lib/internal/ffi/fast-api.js index 486a119a2e07..955477ae7e18 100644 --- a/lib/internal/ffi/fast-api.js +++ b/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); }; diff --git a/src/ffi/data.cc b/src/ffi/data.cc index 73b575395c8c..6a8d54ca0d39 100644 --- a/src/ffi/data.cc +++ b/src/ffi/data.cc @@ -685,7 +685,7 @@ void ExportBytes(const FunctionCallbackInfo& 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& args) { std::shared_ptr store; if (args[0]->IsArrayBuffer()) { - store = args[0].As()->GetBackingStore(); + Local buffer = args[0].As(); + 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()->GetBackingStore(); } else if (args[0]->IsArrayBufferView()) { + Local view = args[0].As(); + 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()->Buffer()->GetBackingStore(); - offset = args[0].As()->ByteOffset(); + store = view->Buffer()->GetBackingStore(); + offset = view->ByteOffset(); } else { THROW_ERR_INVALID_ARG_TYPE( env, diff --git a/src/ffi/fast.cc b/src/ffi/fast.cc index 80ee49e08b28..919bacf061a8 100644 --- a/src/ffi/fast.cc +++ b/src/ffi/fast.cc @@ -246,18 +246,45 @@ extern "C" uintptr_t node_ffi_fast_buffer_data(v8::Local value, // returns zero after throwing, preventing the native target from seeing an // invalid pointer value. constexpr uintptr_t kInvalidBuffer = std::numeric_limits::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 view = value.As(); + 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()->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. diff --git a/src/ffi/types.cc b/src/ffi/types.cc index 9ba3cc4da448..db0c913c547d 100644 --- a/src/ffi/types.cc +++ b/src/ffi/types.cc @@ -700,6 +700,14 @@ Maybe ToFFIArgument(Environment* env, // invalidating that backing store during the active FFI call is // unsupported and dangerous. Local view = arg.As(); + 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 store = view->Buffer()->GetBackingStore(); if (!store) { @@ -721,6 +729,11 @@ Maybe ToFFIArgument(Environment* env, // that backing store during the active FFI call is unsupported and // dangerous. Local buffer = arg.As(); + if (buffer->WasDetached()) { + THROW_ERR_INVALID_ARG_VALUE( + env, "Argument %u is a detached ArrayBuffer", index); + return {}; + } std::shared_ptr store = buffer->GetBackingStore(); if (!store) { diff --git a/test/ffi/test-ffi-fast-buffer.js b/test/ffi/test-ffi-fast-buffer.js index e4399ee8ee47..a6b196efc014 100644 --- a/test/ffi/test-ffi-fast-buffer.js +++ b/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(); diff --git a/test/ffi/test-ffi-memory.js b/test/ffi/test-ffi-memory.js index f17f56c410f8..ee88e9ef4151 100644 --- a/test/ffi/test-ffi-memory.js +++ b/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);