GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — ffi: reject detached ArrayBuffers as pointers · nodejs/node@be2ab89

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

lib/internal/ffi
- fast-api.js
src/ffi
test/ffi
- test-ffi-fast-buffer.js
- test-ffi-memory.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

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

Copy file name to clipboardExpand all lines: lib/internal/ffi/fast-api.js

+29-2Lines changed: 29 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -1,13 +1,16 @@`
`1`	`1`	`'use strict';`
`2`	`2`
`3`	`3`	`const {`
	`4`	`+ ArrayBufferPrototypeGetDetached,`
`4`	`5`	`ArrayPrototypeIncludes,`
	`6`	`+ DataViewPrototypeGetBuffer,`
`5`	`7`	`NumberIsInteger,`
`6`	`8`	`ObjectDefineProperty,`
`7`	`9`	`ReflectApply,`
`8`	`10`	`SafeWeakMap,`
`9`	`11`	`StringPrototypeIncludes,`
`10`	`12`	`TypeError,`
	`13`	`+ TypedArrayPrototypeGetBuffer,`
`11`	`14`	`} = primordials;`
`12`	`15`
`13`	`16`	`const {`
`@@ -16,7 +19,9 @@ const {`
`16`	`19`
`17`	`20`	`const {`
`18`	`21`	`isAnyArrayBuffer,`
	`22`	`+ isArrayBuffer,`
`19`	`23`	`isArrayBufferView,`
	`24`	`+ isDataView,`
`20`	`25`	`} = require('internal/util/types');`
`21`	`26`
`22`	`27`	`const {`
`@@ -167,6 +172,28 @@ function getStringConversionPointer(state, value, index) {`
`167`	`172`	`return entry.pointer;`
`168`	`173`	`}`
`169`	`174`
	`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`	`+`
`170`	`197`	`function convertPointerArg(type, value, stringState, index) {`
`171`	`198`	`validateFastPointerArg(type, value, index);`
`172`	`199`	`if (needsNullPointerConversion(type) &&`
`@@ -177,7 +204,7 @@ function convertPointerArg(type, value, stringState, index) {`
`177`	`204`	`return getStringConversionPointer(stringState, value, index);`
`178`	`205`	`}`
`179`	`206`	`if (hasPointerMemoryArg(type, value)) {`
`180`		`- return getRawPointer(value);`
	`207`	`+ return getRawPointerArg(value, index);`
`181`	`208`	`}`
`182`	`209`	`// Pointer-like values (e.g. BigInt addresses) are passed through, matching`
`183`	`210`	`// ToFFIArgument in src/ffi/types.cc and the single-argument fast path.`
`@@ -287,7 +314,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {`
`287`	`314`	`if (fastBufferInvoke !== undefined) {`
`288`	`315`	`return fastBufferInvoke(arg);`
`289`	`316`	`}`
`290`		`- arg = getRawPointer(arg);`
	`317`	`+ arg = getRawPointerArg(arg, 0);`
`291`	`318`	`}`
`292`	`319`	`return rawFn(arg);`
`293`	`320`	`};`

Collapse file

`‎src/ffi/data.cc‎`

Copy file name to clipboardExpand all lines: src/ffi/data.cc

+15-4Lines changed: 15 additions & 4 deletions

Original file line number	Diff line number	Diff line change
`@@ -685,7 +685,7 @@ void ExportBytes(const FunctionCallbackInfo<Value>& args) {`
`685`	`685`	`args[0]->IsArrayBufferView()) {`
`686`	`686`	`view.ReadValue(args[0]);`
`687`	`687`	`if (view.WasDetached()) {`
`688`		`- THROW_ERR_INVALID_ARG_VALUE(env, "Invalid ArrayBufferView backing store");`
	`688`	`+ THROW_ERR_INVALID_ARG_VALUE(env, "ArrayBuffer is detached");`
`689`	`689`	`return;`
`690`	`690`	`}`
`691`	`691`	`} else {`
`@@ -749,15 +749,26 @@ void GetRawPointer(const FunctionCallbackInfo<Value>& args) {`
`749`	`749`	`std::shared_ptr<BackingStore> store;`
`750`	`750`
`751`	`751`	`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();`
`753`	`758`	`} else if (args[0]->IsSharedArrayBuffer()) {`
`754`	`759`	`store = args[0].As<SharedArrayBuffer>()->GetBackingStore();`
`755`	`760`	`} 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`	`+ }`
`756`	`767`	`// Access the store here to ensure that it exists. Small typed arrays`
`757`	`768`	`// may not have a store until this point and can instead be stored`
`758`	`769`	`// 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();`
`761`	`772`	`} else {`
`762`	`773`	`THROW_ERR_INVALID_ARG_TYPE(`
`763`	`774`	`env,`

Collapse file

`‎src/ffi/fast.cc‎`

Copy file name to clipboardExpand all lines: src/ffi/fast.cc

+29-2Lines changed: 29 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -246,18 +246,45 @@ extern "C" uintptr_t node_ffi_fast_buffer_data(v8::Local<v8::Value> value,`
`246`	`246`	`// returns zero after throwing, preventing the native target from seeing an`
`247`	`247`	`// invalid pointer value.`
`248`	`248`	`constexpr uintptr_t kInvalidBuffer = std::numeric_limits<uintptr_t>::max();`
	`249`	`+ v8::Isolate* isolate = options != nullptr ? options->isolate : nullptr;`
`249`	`250`
`250`	`251`	`// Accept only memory-backed JS values in the native helper. Other pointer`
`251`	`252`	`// conversions, including strings, stay in the JS wrapper so their temporary`
`252`	`253`	`// lifetime is explicit.`
`253`	`254`	`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`	`+ }`
`254`	`269`	`return PointerFromValue(value);`
`255`	`270`	`}`
`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()) {`
`257`	`285`	`return PointerFromValue(value);`
`258`	`286`	`}`
`259`	`287`
`260`		`- v8::Isolate* isolate = options != nullptr ? options->isolate : nullptr;`
`261`	`288`	`if (isolate != nullptr) {`
`262`	`289`	`// No HandleScope is active during a Fast API call, so open one before`
`263`	`290`	`// creating the error object.`

Collapse file

`‎src/ffi/types.cc‎`

Copy file name to clipboardExpand all lines: src/ffi/types.cc

+13Lines changed: 13 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -700,6 +700,14 @@ Maybe<FFIArgumentCategory> ToFFIArgument(Environment* env,`
`700`	`700`	`// invalidating that backing store during the active FFI call is`
`701`	`701`	`// unsupported and dangerous.`
`702`	`702`	`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`	`+ }`
`703`	`711`	`std::shared_ptr<BackingStore> store = view->Buffer()->GetBackingStore();`
`704`	`712`
`705`	`713`	`if (!store) {`
`@@ -721,6 +729,11 @@ Maybe<FFIArgumentCategory> ToFFIArgument(Environment* env,`
`721`	`729`	`// that backing store during the active FFI call is unsupported and`
`722`	`730`	`// dangerous.`
`723`	`731`	`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`	`+ }`
`724`	`737`	`std::shared_ptr<BackingStore> store = buffer->GetBackingStore();`
`725`	`738`
`726`	`739`	`if (!store) {`

Collapse file

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

Copy file name to clipboardExpand all lines: test/ffi/test-ffi-fast-buffer.js

+26-1Lines changed: 26 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -98,6 +98,10 @@ test('fast FFI string buffers survive reentrant callbacks', {`
`98`	`98`
`99`	`99`	`test('optimized buffer signatures preserve pointer-like conversions', () => {`
`100`	`100`	`const lib = new ffi.DynamicLibrary(libraryPath);`
	`101`	`+ const asPointer = lib.getFunction('pointer_to_usize', {`
	`102`	`+ arguments: ['pointer'],`
	`103`	`+ return: 'u64',`
	`104`	`+ });`
`101`	`105`	`const asBuffer = lib.getFunction('pointer_to_usize', {`
`102`	`106`	`arguments: ['buffer'],`
`103`	`107`	`return: 'u64',`
`@@ -107,6 +111,10 @@ test('optimized buffer signatures preserve pointer-like conversions', () => {`
`107`	`111`	`return: 'u64',`
`108`	`112`	`});`
`109`	`113`
	`114`	`+ function callPointer(value) {`
	`115`	`+ return asPointer(value);`
	`116`	`+ }`
	`117`	`+`
`110`	`118`	`function callBuffer(value) {`
`111`	`119`	`return asBuffer(value);`
`112`	`120`	`}`
`@@ -117,17 +125,34 @@ test('optimized buffer signatures preserve pointer-like conversions', () => {`
`117`	`125`
`118`	`126`	`try {`
`119`	`127`	`for (let i = 0; i < 100_000; i++) {`
	`128`	`+ assert.strictEqual(callPointer(0n), 0n);`
`120`	`129`	`assert.strictEqual(callBuffer(0n), 0n);`
`121`	`130`	`assert.strictEqual(callArrayBuffer(0n), 0n);`
`122`	`131`	`}`
`123`	`132`
`124`		`- for (const call of [callBuffer, callArrayBuffer]) {`
	`133`	`+ for (const call of [callPointer, callBuffer, callArrayBuffer]) {`
`125`	`134`	`assert.strictEqual(call(null), 0n);`
`126`	`135`	`assert.strictEqual(call(undefined), 0n);`
`127`	`136`	`assert.notStrictEqual(call('ffi'), 0n);`
`128`	`137`
`129`	`138`	`const bytes = Buffer.alloc(1);`
`130`	`139`	`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`	`+ }`
`131`	`156`	`}`
`132`	`157`	`} finally {`
`133`	`158`	`lib.close();`

Collapse file

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

Copy file name to clipboardExpand all lines: test/ffi/test-ffi-memory.js

+35Lines changed: 35 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -146,6 +146,41 @@ test('ffi getRawPointer returns raw addresses for byte sources', () => {`
`146`	`146`	`assert.strictEqual(sharedViewPointer, sharedArrayBufferPointer + 2n);`
`147`	`147`	`});`
`148`	`148`
	`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`	`+`
`149`	`184`	`test('ffi exportString and exportBuffer copy data into native memory', () => {`
`150`	`185`	`withAllocations(common.mustCall((alloc) => {`
`151`	`186`	`const stringPtr = alloc(16);`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.