GeezerVison is showing github.com — ffi: validate fast integer argument ranges · nodejs/node@0fb1d2b
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/ffi/fast-api.js +58-10Lines changed: 58 additions & 10 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 2 | 2 |
| 3 | 3 | const { | 4 | 4 | ArrayPrototypeIncludes, | | 5 | + NumberIsInteger, | 5 | 6 | ObjectDefineProperty, | 6 | 7 | ReflectApply, | 7 | 8 | StringPrototypeIncludes, | @@ -19,6 +20,7 @@ const { | 19 | 20 | } = require('internal/util/types'); | 20 | 21 |
| 21 | 22 | const { | | 23 | + charIsSigned, | 22 | 24 | getRawPointer, | 23 | 25 | kFastArguments, | 24 | 26 | kFastBufferInvoke, | @@ -28,6 +30,33 @@ const { | 28 | 30 | const kFastBuffer = Symbol('kFastBuffer'); | 29 | 31 | const kStringConversionBuffer = Symbol('kStringConversionBuffer'); | 30 | 32 |
| | 33 | +const U64_MAX = 0xFFFFFFFFFFFFFFFFn; | | 34 | +const I64_MAX = 0x7FFFFFFFFFFFFFFFn; | | 35 | +const I64_MIN = -0x8000000000000000n; | | 36 | + | | 37 | +// These ranges mirror ToFFIArgument in src/ffi/types.cc. V8's Fast API | | 38 | +// exposes narrow integers as 32-bit values and uses truncating BigInt | | 39 | +// conversions, so the public FFI ranges must be checked before the raw call. | | 40 | +const fastIntegerTypeInfo = { | | 41 | + __proto__: null, | | 42 | + i8: { kind: 'number', min: -128, max: 127, label: 'an int8' }, | | 43 | + int8: { kind: 'number', min: -128, max: 127, label: 'an int8' }, | | 44 | + char: charIsSigned ? | | 45 | + { kind: 'number', min: -128, max: 127, label: 'an int8' } : | | 46 | + { kind: 'number', min: 0, max: 255, label: 'a uint8' }, | | 47 | + u8: { kind: 'number', min: 0, max: 255, label: 'a uint8' }, | | 48 | + uint8: { kind: 'number', min: 0, max: 255, label: 'a uint8' }, | | 49 | + bool: { kind: 'number', min: 0, max: 255, label: 'a uint8' }, | | 50 | + i16: { kind: 'number', min: -32768, max: 32767, label: 'an int16' }, | | 51 | + int16: { kind: 'number', min: -32768, max: 32767, label: 'an int16' }, | | 52 | + u16: { kind: 'number', min: 0, max: 65535, label: 'a uint16' }, | | 53 | + uint16: { kind: 'number', min: 0, max: 65535, label: 'a uint16' }, | | 54 | + i64: { kind: 'bigint', min: I64_MIN, max: I64_MAX, label: 'an int64' }, | | 55 | + int64: { kind: 'bigint', min: I64_MIN, max: I64_MAX, label: 'an int64' }, | | 56 | + u64: { kind: 'bigint', min: 0n, max: U64_MAX, label: 'a uint64' }, | | 57 | + uint64: { kind: 'bigint', min: 0n, max: U64_MAX, label: 'a uint64' }, | | 58 | +}; | | 59 | + | 31 | 60 | function throwFFIArgError(msg) { | 32 | 61 | // eslint-disable-next-line no-restricted-syntax | 33 | 62 | const err = new TypeError(msg); | @@ -40,6 +69,17 @@ function throwFFIArgCountError(expected, actual) { | 40 | 69 | `Invalid argument count: expected ${expected}, got ${actual}`); | 41 | 70 | } | 42 | 71 |
| | 72 | +function validateFastIntegerArg(type, value, index) { | | 73 | + const info = fastIntegerTypeInfo[type]; | | 74 | + if (info === undefined) return; | | 75 | + const validType = info.kind === 'number' ? | | 76 | + typeof value === 'number' && NumberIsInteger(value) : | | 77 | + typeof value === 'bigint'; | | 78 | + if (!validType || value < info.min || value > info.max) { | | 79 | + throwFFIArgError(`Argument ${index} must be ${info.label}`); | | 80 | + } | | 81 | +} | | 82 | + | 43 | 83 | function needsRawPointerConversion(type, rawFn) { | 44 | 84 | if (rawFn !== undefined && rawFn[kFastBuffer] === true && | 45 | 85 | (type === 'buffer' || type === 'arraybuffer')) { | @@ -134,10 +174,11 @@ function convertPointerArg(type, value, owner, index) { | 134 | 174 | return value; | 135 | 175 | } | 136 | 176 |
| 137 | | -function getPointerConversionIndexes(argumentsTypes, rawFn) { | | 177 | +function getFastArgumentIndexes(argumentsTypes, rawFn) { | 138 | 178 | let indexes = null; | 139 | 179 | for (let i = 0; i < argumentsTypes.length; i++) { | 140 | | - if (!needsPointerConversion(argumentsTypes[i], rawFn)) { | | 180 | + if (fastIntegerTypeInfo[argumentsTypes[i]] === undefined && | | 181 | + !needsPointerConversion(argumentsTypes[i], rawFn)) { | 141 | 182 | continue; | 142 | 183 | } | 143 | 184 | if (indexes === null) { | @@ -148,6 +189,12 @@ function getPointerConversionIndexes(argumentsTypes, rawFn) { | 148 | 189 | return indexes; | 149 | 190 | } | 150 | 191 |
| | 192 | +function convertFastArg(type, value, rawFn, owner, index) { | | 193 | + validateFastIntegerArg(type, value, index); | | 194 | + return needsPointerConversion(type, rawFn) ? | | 195 | + convertPointerArg(type, value, owner, index) : value; | | 196 | +} | | 197 | + | 151 | 198 | function initializeFastBufferMetadata(rawFn, argumentTypes) { | 152 | 199 | if (rawFn === undefined || rawFn === null || argumentTypes === undefined) { | 153 | 200 | return; | @@ -192,7 +239,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { | 192 | 239 | return rawFn; | 193 | 240 | } | 194 | 241 |
| 195 | | - const indexes = getPointerConversionIndexes(argumentTypes, rawFn); | | 242 | + const indexes = getFastArgumentIndexes(argumentTypes, rawFn); | 196 | 243 | if (indexes === null) { | 197 | 244 | return rawFn; | 198 | 245 | } | @@ -209,6 +256,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { | 209 | 256 | if (arguments.length !== 1) { | 210 | 257 | throwFFIArgCountError(1, arguments.length); | 211 | 258 | } | | 259 | + validateFastIntegerArg(t0, a0, 0); | 212 | 260 | let arg = a0; | 213 | 261 | if (needsNullPointerConversion(t0) && | 214 | 262 | (arg === null || arg === undefined)) { | @@ -232,8 +280,8 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { | 232 | 280 | if (arguments.length !== 2) { | 233 | 281 | throwFFIArgCountError(2, arguments.length); | 234 | 282 | } | 235 | | - return rawFn(c0 ? convertPointerArg(t0, a0, owner, 0) : a0, | 236 | | - c1 ? convertPointerArg(t1, a1, owner, 1) : a1); | | 283 | + return rawFn(c0 ? convertFastArg(t0, a0, rawFn, owner, 0) : a0, | | 284 | + c1 ? convertFastArg(t1, a1, rawFn, owner, 1) : a1); | 237 | 285 | }; | 238 | 286 | } else if (nargs === 3) { | 239 | 287 | const c0 = ArrayPrototypeIncludes(indexes, 0); | @@ -246,9 +294,9 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { | 246 | 294 | if (arguments.length !== 3) { | 247 | 295 | throwFFIArgCountError(3, arguments.length); | 248 | 296 | } | 249 | | - return rawFn(c0 ? convertPointerArg(t0, a0, owner, 0) : a0, | 250 | | - c1 ? convertPointerArg(t1, a1, owner, 1) : a1, | 251 | | - c2 ? convertPointerArg(t2, a2, owner, 2) : a2); | | 297 | + return rawFn(c0 ? convertFastArg(t0, a0, rawFn, owner, 0) : a0, | | 298 | + c1 ? convertFastArg(t1, a1, rawFn, owner, 1) : a1, | | 299 | + c2 ? convertFastArg(t2, a2, rawFn, owner, 2) : a2); | 252 | 300 | }; | 253 | 301 | } else { | 254 | 302 | wrapper = function(...args) { | @@ -257,8 +305,8 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { | 257 | 305 | } | 258 | 306 | for (let i = 0; i < indexes.length; i++) { | 259 | 307 | const index = indexes[i]; | 260 | | - args[index] = convertPointerArg( | 261 | | - argumentTypes[index], args[index], owner, index); | | 308 | + args[index] = convertFastArg( | | 309 | + argumentTypes[index], args[index], rawFn, owner, index); | 262 | 310 | } | 263 | 311 | return ReflectApply(rawFn, undefined, args); | 264 | 312 | }; | |
Collapse file Copy file name to clipboardExpand all lines: src/ffi/fast.cc +14Lines changed: 14 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -160,6 +160,20 @@ bool SignatureNeedsRawPointerConversions(const FFIFunction& fn) { | 160 | 160 | return false; | 161 | 161 | } | 162 | 162 |
| | 163 | +bool SignatureNeedsFastIntegerValidation(const FFIFunction& fn) { | | 164 | + // V8 widens narrow integers to 32 bits and truncates BigInts to 64 bits for | | 165 | + // Fast API calls. These types need a JS range check before the trampoline. | | 166 | + for (const std::string& name : fn.arg_type_names) { | | 167 | + if (name == "bool" || name == "char" || name == "i8" || name == "int8" || | | 168 | + name == "u8" || name == "uint8" || name == "i16" || name == "int16" || | | 169 | + name == "u16" || name == "uint16" || name == "i64" || name == "int64" || | | 170 | + name == "u64" || name == "uint64") { | | 171 | + return true; | | 172 | + } | | 173 | + } | | 174 | + return false; | | 175 | +} | | 176 | + | 163 | 177 | bool IsPointerTypeName(const std::string& name) { | 164 | 178 | // `pointer`, `ptr`, and `function` all use the same uintptr ABI slot; only | 165 | 179 | // the public type spelling differs. | |
Collapse file Copy file name to clipboardExpand all lines: src/ffi/fast.h +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -59,6 +59,7 @@ struct FastFFIMetadata { | 59 | 59 | bool IsFastCallSupported(); | 60 | 60 |
| 61 | 61 | bool SignatureNeedsRawPointerConversions(const FFIFunction& fn); | | 62 | +bool SignatureNeedsFastIntegerValidation(const FFIFunction& fn); | 62 | 63 | bool IsPointerTypeName(const std::string& name); | 63 | 64 | bool SignatureNeedsFastBufferInvoke(const FFIFunction& fn); | 64 | 65 | std::shared_ptr<FFIFunction> CloneWithFastBufferArgNames( | |
Collapse file Copy file name to clipboardExpand all lines: src/node_ffi.cc +6-5Lines changed: 6 additions & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -253,10 +253,11 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction( | 253 | 253 | bool use_fast_api = info->fast_metadata != nullptr; | 254 | 254 | bool use_sb = !use_fast_api && IsSBEligibleSignature(*fn); | 255 | 255 | bool has_ptr_args = use_sb && SignatureHasPointerArgs(*fn); | 256 | | - // Fast API signatures that still accept JS pointer-like values need a JS | 257 | | - // wrapper with the native type names attached as hidden metadata. | 258 | | - bool needs_raw_pointer_conversions = | 259 | | - use_fast_api && SignatureNeedsRawPointerConversions(*fn); | | 256 | + // Fast API signatures that need JS-side argument conversion or range checks | | 257 | + // use a wrapper with the native type names attached as hidden metadata. | | 258 | + bool needs_fast_argument_wrapper = | | 259 | + use_fast_api && (SignatureNeedsRawPointerConversions(*fn) || | | 260 | + SignatureNeedsFastIntegerValidation(*fn)); | 260 | 261 | // A single pointer-like parameter can get a separate Buffer-aware Fast API | 261 | 262 | // entrypoint so Buffer calls avoid JS pointer extraction. | 262 | 263 | bool needs_fast_buffer_invoke = | @@ -381,7 +382,7 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction( | 381 | 382 | } | 382 | 383 | } | 383 | 384 |
| 384 | | - if (needs_raw_pointer_conversions || needs_fast_buffer_invoke) { | | 385 | + if (needs_fast_argument_wrapper || needs_fast_buffer_invoke) { | 385 | 386 | // Fast API wrappers need only the parameter type names. Result conversion | 386 | 387 | // is still handled by V8's CFunction metadata, unlike the SharedBuffer path | 387 | 388 | // which must also know how to read slot 0. | |
Collapse file Copy file name to clipboard +56Lines changed: 56 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +// Flags: --experimental-ffi --allow-natives-syntax | | 2 | +'use strict'; | | 3 | + | | 4 | +const common = require('../common'); | | 5 | +common.skipIfFFIMissing(); | | 6 | + | | 7 | +const assert = require('node:assert'); | | 8 | +const { test } = require('node:test'); | | 9 | +const ffi = require('node:ffi'); | | 10 | +const { fixtureSymbols, libraryPath } = require('./ffi-test-common'); | | 11 | + | | 12 | +function optimize(fn, value) { | | 13 | + eval('%PrepareFunctionForOptimization(fn)'); | | 14 | + fn(value); | | 15 | + fn(value); | | 16 | + eval('%OptimizeFunctionOnNextCall(fn)'); | | 17 | + fn(value); | | 18 | +} | | 19 | + | | 20 | +test('fast FFI validates integer argument ranges', () => { | | 21 | + const { lib, functions } = ffi.dlopen(libraryPath, fixtureSymbols); | | 22 | + try { | | 23 | + function callI8(value) { return functions.add_i8(value, 0); } | | 24 | + | | 25 | + function callU8(value) { return functions.add_u8(value, 0); } | | 26 | + | | 27 | + function callI16(value) { return functions.add_i16(value, 0); } | | 28 | + | | 29 | + function callU16(value) { return functions.add_u16(value, 0); } | | 30 | + | | 31 | + function callI64(value) { return functions.add_i64(value, 0n); } | | 32 | + | | 33 | + function callU64(value) { return functions.add_u64(value, 0n); } | | 34 | + | | 35 | + for (const [fn, value] of [ | | 36 | + [callI8, 0], | | 37 | + [callU8, 0], | | 38 | + [callI16, 0], | | 39 | + [callU16, 0], | | 40 | + [callI64, 0n], | | 41 | + [callU64, 0n], | | 42 | + ]) { | | 43 | + optimize(fn, value); | | 44 | + } | | 45 | + | | 46 | + const expect = { code: 'ERR_INVALID_ARG_VALUE' }; | | 47 | + assert.throws(() => callI8(128), expect); | | 48 | + assert.throws(() => callU8(256), expect); | | 49 | + assert.throws(() => callI16(32768), expect); | | 50 | + assert.throws(() => callU16(65536), expect); | | 51 | + assert.throws(() => callI64(2n ** 63n), expect); | | 52 | + assert.throws(() => callU64(2n ** 64n), expect); | | 53 | + } finally { | | 54 | + lib.close(); | | 55 | + } | | 56 | +}); |
|