Skip to content

Commit 0fb1d2b

Browse files
trivikraduh95
authored andcommitted
ffi: validate fast integer argument ranges
Validate narrow integer and 64-bit BigInt arguments before entering the Fast API trampoline. This prevents out-of-range values from being silently truncated or wrapped and matches the generic FFI path. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.6-sol PR-URL: #64614 Fixes: #64613 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent bbd6fc5 commit 0fb1d2b

5 files changed

Lines changed: 135 additions & 15 deletions

File tree

lib/internal/ffi/fast-api.js

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
ArrayPrototypeIncludes,
5+
NumberIsInteger,
56
ObjectDefineProperty,
67
ReflectApply,
78
StringPrototypeIncludes,
@@ -19,6 +20,7 @@ const {
1920
} = require('internal/util/types');
2021

2122
const {
23+
charIsSigned,
2224
getRawPointer,
2325
kFastArguments,
2426
kFastBufferInvoke,
@@ -28,6 +30,33 @@ const {
2830
const kFastBuffer = Symbol('kFastBuffer');
2931
const kStringConversionBuffer = Symbol('kStringConversionBuffer');
3032

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+
3160
function throwFFIArgError(msg) {
3261
// eslint-disable-next-line no-restricted-syntax
3362
const err = new TypeError(msg);
@@ -40,6 +69,17 @@ function throwFFIArgCountError(expected, actual) {
4069
`Invalid argument count: expected ${expected}, got ${actual}`);
4170
}
4271

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+
4383
function needsRawPointerConversion(type, rawFn) {
4484
if (rawFn !== undefined && rawFn[kFastBuffer] === true &&
4585
(type === 'buffer' || type === 'arraybuffer')) {
@@ -134,10 +174,11 @@ function convertPointerArg(type, value, owner, index) {
134174
return value;
135175
}
136176

137-
function getPointerConversionIndexes(argumentsTypes, rawFn) {
177+
function getFastArgumentIndexes(argumentsTypes, rawFn) {
138178
let indexes = null;
139179
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)) {
141182
continue;
142183
}
143184
if (indexes === null) {
@@ -148,6 +189,12 @@ function getPointerConversionIndexes(argumentsTypes, rawFn) {
148189
return indexes;
149190
}
150191

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+
151198
function initializeFastBufferMetadata(rawFn, argumentTypes) {
152199
if (rawFn === undefined || rawFn === null || argumentTypes === undefined) {
153200
return;
@@ -192,7 +239,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
192239
return rawFn;
193240
}
194241

195-
const indexes = getPointerConversionIndexes(argumentTypes, rawFn);
242+
const indexes = getFastArgumentIndexes(argumentTypes, rawFn);
196243
if (indexes === null) {
197244
return rawFn;
198245
}
@@ -209,6 +256,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
209256
if (arguments.length !== 1) {
210257
throwFFIArgCountError(1, arguments.length);
211258
}
259+
validateFastIntegerArg(t0, a0, 0);
212260
let arg = a0;
213261
if (needsNullPointerConversion(t0) &&
214262
(arg === null || arg === undefined)) {
@@ -232,8 +280,8 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
232280
if (arguments.length !== 2) {
233281
throwFFIArgCountError(2, arguments.length);
234282
}
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);
237285
};
238286
} else if (nargs === 3) {
239287
const c0 = ArrayPrototypeIncludes(indexes, 0);
@@ -246,9 +294,9 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
246294
if (arguments.length !== 3) {
247295
throwFFIArgCountError(3, arguments.length);
248296
}
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);
252300
};
253301
} else {
254302
wrapper = function(...args) {
@@ -257,8 +305,8 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
257305
}
258306
for (let i = 0; i < indexes.length; i++) {
259307
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);
262310
}
263311
return ReflectApply(rawFn, undefined, args);
264312
};

src/ffi/fast.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,20 @@ bool SignatureNeedsRawPointerConversions(const FFIFunction& fn) {
160160
return false;
161161
}
162162

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+
163177
bool IsPointerTypeName(const std::string& name) {
164178
// `pointer`, `ptr`, and `function` all use the same uintptr ABI slot; only
165179
// the public type spelling differs.

src/ffi/fast.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ struct FastFFIMetadata {
5959
bool IsFastCallSupported();
6060

6161
bool SignatureNeedsRawPointerConversions(const FFIFunction& fn);
62+
bool SignatureNeedsFastIntegerValidation(const FFIFunction& fn);
6263
bool IsPointerTypeName(const std::string& name);
6364
bool SignatureNeedsFastBufferInvoke(const FFIFunction& fn);
6465
std::shared_ptr<FFIFunction> CloneWithFastBufferArgNames(

src/node_ffi.cc

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,11 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
253253
bool use_fast_api = info->fast_metadata != nullptr;
254254
bool use_sb = !use_fast_api && IsSBEligibleSignature(*fn);
255255
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));
260261
// A single pointer-like parameter can get a separate Buffer-aware Fast API
261262
// entrypoint so Buffer calls avoid JS pointer extraction.
262263
bool needs_fast_buffer_invoke =
@@ -381,7 +382,7 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
381382
}
382383
}
383384

384-
if (needs_raw_pointer_conversions || needs_fast_buffer_invoke) {
385+
if (needs_fast_argument_wrapper || needs_fast_buffer_invoke) {
385386
// Fast API wrappers need only the parameter type names. Result conversion
386387
// is still handled by V8's CFunction metadata, unlike the SharedBuffer path
387388
// which must also know how to read slot 0.
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
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+
});

0 commit comments

Comments
 (0)