FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

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

/ node Public

Commit be2ab89

Browse files
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
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
'use strict';
22

33
const {
4+
ArrayBufferPrototypeGetDetached,
45
ArrayPrototypeIncludes,
6+
DataViewPrototypeGetBuffer,
57
NumberIsInteger,
68
ObjectDefineProperty,
79
ReflectApply,
810
SafeWeakMap,
911
StringPrototypeIncludes,
1012
TypeError,
13+
TypedArrayPrototypeGetBuffer,
1114
} = primordials;
1215

1316
const {
@@ -16,7 +19,9 @@ const {
1619

1720
const {
1821
isAnyArrayBuffer,
22+
isArrayBuffer,
1923
isArrayBufferView,
24+
isDataView,
2025
} = require('internal/util/types');
2126

2227
const {
@@ -167,6 +172,28 @@ function getStringConversionPointer(state, value, index) {
167172
return entry.pointer;
168173
}
169174

175+
function getRawPointerArg(value, index) {
176+
let buffer;
177+
let isView = false;
178+
if (isArrayBuffer(value)) {
179+
buffer = value;
180+
} else if (isArrayBufferView(value)) {
181+
isView = true;
182+
buffer = isDataView(value) ?
183+
DataViewPrototypeGetBuffer(value) :
184+
TypedArrayPrototypeGetBuffer(value);
185+
}
186+
187+
if (buffer !== undefined && isArrayBuffer(buffer) &&
188+
ArrayBufferPrototypeGetDetached(buffer)) {
189+
throwFFIArgError(isView ?
190+
`Argument ${index} is an ArrayBufferView backed by a detached ArrayBuffer` :
191+
`Argument ${index} is a detached ArrayBuffer`);
192+
}
193+
194+
return getRawPointer(value);
195+
}
196+
170197
function convertPointerArg(type, value, stringState, index) {
171198
validateFastPointerArg(type, value, index);
172199
if (needsNullPointerConversion(type) &&
@@ -177,7 +204,7 @@ function convertPointerArg(type, value, stringState, index) {
177204
return getStringConversionPointer(stringState, value, index);
178205
}
179206
if (hasPointerMemoryArg(type, value)) {
180-
return getRawPointer(value);
207+
return getRawPointerArg(value, index);
181208
}
182209
// Pointer-like values (e.g. BigInt addresses) are passed through, matching
183210
// ToFFIArgument in src/ffi/types.cc and the single-argument fast path.
@@ -287,7 +314,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
287314
if (fastBufferInvoke !== undefined) {
288315
return fastBufferInvoke(arg);
289316
}
290-
arg = getRawPointer(arg);
317+
arg = getRawPointerArg(arg, 0);
291318
}
292319
return rawFn(arg);
293320
};

‎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
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,41 @@ test('ffi getRawPointer returns raw addresses for byte sources', () => {
146146
assert.strictEqual(sharedViewPointer, sharedArrayBufferPointer + 2n);
147147
});
148148

149+
test('ffi rejects detached array buffers and views as pointers', () => {
150+
const arrayBuffer = new ArrayBuffer(8);
151+
const typedArray = new Uint8Array(arrayBuffer);
152+
const dataView = new DataView(arrayBuffer);
153+
154+
arrayBuffer.transfer();
155+
156+
assert.throws(() => ffi.exportArrayBuffer(arrayBuffer, 0n, 0), {
157+
code: 'ERR_INVALID_ARG_VALUE',
158+
message: 'ArrayBuffer is detached',
159+
});
160+
161+
for (const [value, rawPointerMessage, argumentMessage] of [
162+
[
163+
arrayBuffer,
164+
'ArrayBuffer is detached',
165+
'Argument 0 is a detached ArrayBuffer',
166+
],
167+
...[typedArray, dataView].map((view) => [
168+
view,
169+
'ArrayBufferView is backed by a detached ArrayBuffer',
170+
'Argument 0 is an ArrayBufferView backed by a detached ArrayBuffer',
171+
]),
172+
]) {
173+
assert.throws(() => ffi.getRawPointer(value), {
174+
code: 'ERR_INVALID_ARG_VALUE',
175+
message: rawPointerMessage,
176+
});
177+
assert.throws(() => symbols.pointer_to_usize(value), {
178+
code: 'ERR_INVALID_ARG_VALUE',
179+
message: argumentMessage,
180+
});
181+
}
182+
});
183+
149184
test('ffi exportString and exportBuffer copy data into native memory', () => {
150185
withAllocations(common.mustCall((alloc) => {
151186
const stringPtr = alloc(16);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL