ffi: reject detached ArrayBuffers as pointers · nodejs/node@be2ab89 · GitHub
Skip to content

Commit be2ab89

Browse files
trivikraduh95
trivikr
authored and
aduh95
committed
ffi: reject detached ArrayBuffers as pointers
Reject detached ArrayBuffers and ArrayBuffer views in getRawPointer() and FFI pointer argument conversion. This prevents detached backing stores from being silently passed to native functions as null pointers. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol PR-URL: #65083 Fixes: #65082 Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 87ee7bf commit be2ab89

6 files changed

Lines changed: 147 additions & 9 deletions

File tree

‎lib/internal/ffi/fast-api.js‎

Lines changed: 29 additions & 2 deletions

‎src/ffi/data.cc‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ void ExportBytes(const FunctionCallbackInfo<Value>& args) {
685685
args[0]->IsArrayBufferView()) {
686686
view.ReadValue(args[0]);
687687
if (view.WasDetached()) {
688-
THROW_ERR_INVALID_ARG_VALUE(env, "Invalid ArrayBufferView backing store");
688+
THROW_ERR_INVALID_ARG_VALUE(env, "ArrayBuffer is detached");
689689
return;
690690
}
691691
} else {
@@ -749,15 +749,26 @@ void GetRawPointer(const FunctionCallbackInfo<Value>& args) {
749749
std::shared_ptr<BackingStore> store;
750750

751751
if (args[0]->IsArrayBuffer()) {
752-
store = args[0].As<ArrayBuffer>()->GetBackingStore();
752+
Local<ArrayBuffer> buffer = args[0].As<ArrayBuffer>();
753+
if (buffer->WasDetached()) {
754+
THROW_ERR_INVALID_ARG_VALUE(env, "ArrayBuffer is detached");
755+
return;
756+
}
757+
store = buffer->GetBackingStore();
753758
} else if (args[0]->IsSharedArrayBuffer()) {
754759
store = args[0].As<SharedArrayBuffer>()->GetBackingStore();
755760
} else if (args[0]->IsArrayBufferView()) {
761+
Local<ArrayBufferView> view = args[0].As<ArrayBufferView>();
762+
if (view->Buffer()->WasDetached()) {
763+
THROW_ERR_INVALID_ARG_VALUE(
764+
env, "ArrayBufferView is backed by a detached ArrayBuffer");
765+
return;
766+
}
756767
// Access the store here to ensure that it exists. Small typed arrays
757768
// may not have a store until this point and can instead be stored
758769
// entirely in-heap.
759-
store = args[0].As<ArrayBufferView>()->Buffer()->GetBackingStore();
760-
offset = args[0].As<ArrayBufferView>()->ByteOffset();
770+
store = view->Buffer()->GetBackingStore();
771+
offset = view->ByteOffset();
761772
} else {
762773
THROW_ERR_INVALID_ARG_TYPE(
763774
env,

‎src/ffi/fast.cc‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,18 +246,45 @@ extern "C" uintptr_t node_ffi_fast_buffer_data(v8::Local<v8::Value> value,
246246
// returns zero after throwing, preventing the native target from seeing an
247247
// invalid pointer value.
248248
constexpr uintptr_t kInvalidBuffer = std::numeric_limits<uintptr_t>::max();
249+
v8::Isolate* isolate = options != nullptr ? options->isolate : nullptr;
249250

250251
// Accept only memory-backed JS values in the native helper. Other pointer
251252
// conversions, including strings, stay in the JS wrapper so their temporary
252253
// lifetime is explicit.
253254
if (value->IsArrayBufferView()) {
255+
v8::Local<v8::ArrayBufferView> view = value.As<v8::ArrayBufferView>();
256+
if (view->Buffer()->WasDetached()) {
257+
if (isolate != nullptr) {
258+
// No HandleScope is active during a Fast API call, so open one before
259+
// creating the error object.
260+
v8::HandleScope scope(isolate);
261+
THROW_ERR_INVALID_ARG_VALUE(
262+
isolate,
263+
"Argument %u is an ArrayBufferView backed by a detached "
264+
"ArrayBuffer",
265+
index);
266+
}
267+
return kInvalidBuffer;
268+
}
254269
return PointerFromValue(value);
255270
}
256-
if (value->IsArrayBuffer() || value->IsSharedArrayBuffer()) {
271+
if (value->IsArrayBuffer()) {
272+
if (value.As<v8::ArrayBuffer>()->WasDetached()) {
273+
if (isolate != nullptr) {
274+
// No HandleScope is active during a Fast API call, so open one before
275+
// creating the error object.
276+
v8::HandleScope scope(isolate);
277+
THROW_ERR_INVALID_ARG_VALUE(
278+
isolate, "Argument %u is a detached ArrayBuffer", index);
279+
}
280+
return kInvalidBuffer;
281+
}
282+
return PointerFromValue(value);
283+
}
284+
if (value->IsSharedArrayBuffer()) {
257285
return PointerFromValue(value);
258286
}
259287

260-
v8::Isolate* isolate = options != nullptr ? options->isolate : nullptr;
261288
if (isolate != nullptr) {
262289
// No HandleScope is active during a Fast API call, so open one before
263290
// creating the error object.

‎src/ffi/types.cc‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,14 @@ Maybe<FFIArgumentCategory> ToFFIArgument(Environment* env,
700700
// invalidating that backing store during the active FFI call is
701701
// unsupported and dangerous.
702702
Local<ArrayBufferView> view = arg.As<ArrayBufferView>();
703+
if (view->Buffer()->WasDetached()) {
704+
THROW_ERR_INVALID_ARG_VALUE(
705+
env,
706+
"Argument %u is an ArrayBufferView backed by a detached "
707+
"ArrayBuffer",
708+
index);
709+
return {};
710+
}
703711
std::shared_ptr<BackingStore> store = view->Buffer()->GetBackingStore();
704712

705713
if (!store) {
@@ -721,6 +729,11 @@ Maybe<FFIArgumentCategory> ToFFIArgument(Environment* env,
721729
// that backing store during the active FFI call is unsupported and
722730
// dangerous.
723731
Local<ArrayBuffer> buffer = arg.As<ArrayBuffer>();
732+
if (buffer->WasDetached()) {
733+
THROW_ERR_INVALID_ARG_VALUE(
734+
env, "Argument %u is a detached ArrayBuffer", index);
735+
return {};
736+
}
724737
std::shared_ptr<BackingStore> store = buffer->GetBackingStore();
725738

726739
if (!store) {

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ test('fast FFI string buffers survive reentrant callbacks', {
9898

9999
test('optimized buffer signatures preserve pointer-like conversions', () => {
100100
const lib = new ffi.DynamicLibrary(libraryPath);
101+
const asPointer = lib.getFunction('pointer_to_usize', {
102+
arguments: ['pointer'],
103+
return: 'u64',
104+
});
101105
const asBuffer = lib.getFunction('pointer_to_usize', {
102106
arguments: ['buffer'],
103107
return: 'u64',
@@ -107,6 +111,10 @@ test('optimized buffer signatures preserve pointer-like conversions', () => {
107111
return: 'u64',
108112
});
109113

114+
function callPointer(value) {
115+
return asPointer(value);
116+
}
117+
110118
function callBuffer(value) {
111119
return asBuffer(value);
112120
}
@@ -117,17 +125,34 @@ test('optimized buffer signatures preserve pointer-like conversions', () => {
117125

118126
try {
119127
for (let i = 0; i < 100_000; i++) {
128+
assert.strictEqual(callPointer(0n), 0n);
120129
assert.strictEqual(callBuffer(0n), 0n);
121130
assert.strictEqual(callArrayBuffer(0n), 0n);
122131
}
123132

124-
for (const call of [callBuffer, callArrayBuffer]) {
133+
for (const call of [callPointer, callBuffer, callArrayBuffer]) {
125134
assert.strictEqual(call(null), 0n);
126135
assert.strictEqual(call(undefined), 0n);
127136
assert.notStrictEqual(call('ffi'), 0n);
128137

129138
const bytes = Buffer.alloc(1);
130139
assert.strictEqual(call(bytes), ffi.getRawPointer(bytes));
140+
141+
const arrayBuffer = new ArrayBuffer(8);
142+
const typedArray = new Uint8Array(arrayBuffer);
143+
const dataView = new DataView(arrayBuffer);
144+
arrayBuffer.transfer();
145+
146+
assert.throws(() => call(arrayBuffer), {
147+
code: 'ERR_INVALID_ARG_VALUE',
148+
message: 'Argument 0 is a detached ArrayBuffer',
149+
});
150+
for (const view of [typedArray, dataView]) {
151+
assert.throws(() => call(view), {
152+
code: 'ERR_INVALID_ARG_VALUE',
153+
message: 'Argument 0 is an ArrayBufferView backed by a detached ArrayBuffer',
154+
});
155+
}
131156
}
132157
} finally {
133158
lib.close();

‎test/ffi/test-ffi-memory.js‎

Lines changed: 35 additions & 0 deletions

0 commit comments

Comments
 (0)