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

ffi: validate the value passed to the float setters · nodejs/node@1fe7a1c · GitHub

/ node Public

Commit 1fe7a1c

Browse files
soulee-dev
authored and
aduh95
committed
ffi: validate the value passed to the float setters
setInt8() through setUint64() require IsNumber() before any range check and reject anything else with ERR_INVALID_ARG_VALUE, but setFloat32() and setFloat64() call ToNumber() and write whatever it returns, so a string, a boolean or a plain object is converted instead of rejected and a typo such as '1,5' stores NaN in native memory with no error at the call site. The same double is type-checked when it is passed as a call argument: ToFFIArgument() requires IsNumber() and otherwise throws "Argument %s must be a double". The coercion also discards a pending exception. When ToNumber() fails because the value has a valueOf() that throws, the branch throws ERR_INVALID_ARG_VALUE on top of the exception V8 has already scheduled, so the original error never reaches the caller, whereas DataView.prototype.setFloat64() and Buffer.prototype.writeDoubleLE() both propagate it. Check IsNumber() instead, matching the wording of the integer setters and of ToFFIArgument(). The check runs before any conversion, so valueOf() is never invoked and there is no pending exception left to discard. This was the only ToNumber(context) call in src/. Signed-off-by: Soul Lee <alus20x@gmail.com> PR-URL: #65342 Fixes: #65341 Refs: #62858 Refs: #64614 Refs: #64691 Refs: #65032 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
1 parent 9572a0b commit 1fe7a1c

2 files changed

Lines changed: 13 additions & 9 deletions

File tree

‎src/ffi/data.cc‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,13 @@ using v8::ArrayBuffer;
1616
using v8::ArrayBufferView;
1717
using v8::BackingStore;
1818
using v8::BigInt;
19-
using v8::Context;
2019
using v8::FunctionCallbackInfo;
2120
using v8::Integer;
2221
using v8::Isolate;
2322
using v8::Just;
2423
using v8::JustVoid;
2524
using v8::Local;
2625
using v8::Maybe;
27-
using v8::MaybeLocal;
2826
using v8::NewStringType;
2927
using v8::Nothing;
3028
using v8::Number;
@@ -312,7 +310,6 @@ void SetValue(const FunctionCallbackInfo<Value>& args) {
312310
}
313311

314312
T converted;
315-
Local<Context> context = env->context();
316313

317314
if constexpr (std::is_same_v<T, int8_t>) {
318315
int64_t validated;
@@ -400,15 +397,16 @@ void SetValue(const FunctionCallbackInfo<Value>& args) {
400397
return;
401398
}
402399
} else if constexpr (std::is_same_v<T, float> || std::is_same_v<T, double>) {
403-
MaybeLocal<Number> number = value->ToNumber(context);
404-
Local<Number> number_local;
405-
406-
if (!number.ToLocal(&number_local)) {
407-
THROW_ERR_INVALID_ARG_VALUE(env, "Value must be a number");
400+
if (!value->IsNumber()) {
401+
if constexpr (std::is_same_v<T, float>) {
402+
THROW_ERR_INVALID_ARG_VALUE(env, "Value must be a float");
403+
} else {
404+
THROW_ERR_INVALID_ARG_VALUE(env, "Value must be a double");
405+
}
408406
return;
409407
}
410408

411-
converted = static_cast<T>(number_local->Value());
409+
converted = static_cast<T>(value.As<Number>()->Value());
412410
} else {
413411
UNREACHABLE();
414412
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,12 @@ test('ffi validates memory access arguments', () => {
273273
assert.throws(() => ffi.setUint64(ptr, 0, -1n), /Value must be a uint64/);
274274
assert.throws(() => ffi.setUint64(ptr, 0, 2n ** 64n), /Value must be a uint64/);
275275
assert.throws(() => ffi.setUint64(ptr, 0, Number.MAX_SAFE_INTEGER + 1), /Value must be a uint64/);
276+
assert.throws(() => ffi.setFloat32(ptr, 0, '1.5'), /Value must be a float/);
277+
assert.throws(() => ffi.setFloat32(ptr, 0, null), /Value must be a float/);
278+
assert.throws(() => ffi.setFloat64(ptr, 0, '1.5'), /Value must be a double/);
279+
assert.throws(() => ffi.setFloat64(ptr, 0, true), /Value must be a double/);
280+
assert.throws(() => ffi.setFloat64(ptr, 0, {}), /Value must be a double/);
281+
assert.throws(() => ffi.setFloat64(ptr, 0, { valueOf: common.mustNotCall() }), /Value must be a double/);
276282
assert.throws(() => ffi.exportString(1, ptr, 4), { code: 'ERR_INVALID_ARG_TYPE' });
277283
assert.throws(() => ffi.exportString('ok', ptr, -1), { code: 'ERR_OUT_OF_RANGE' });
278284
assert.throws(() => ffi.exportString('ok', ptr, 4, 1), { code: 'ERR_INVALID_ARG_TYPE' });

0 commit comments

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