Repository navigation
n-api: napi_get_value_int64() needs a strict version #26323
Description
I am working on several n-api modules.
We need to enforce that integer arguments are really integers.
This is critical for avoiding buffer overflow and dangerous behavior.
Unfortunately, at present, napi_get_value_int64() and friends are lax.
For example, from the documentation:
Non-finite number values (NaN, +Infinity, or -Infinity) set the result to zero.
I can understand this from a performance perspective, and have been involved in these performance discussions before, but there should also be a way to test for non-integers (including floats), e.g. napi_get_value_int64_strict().
I don't know if this already exists?
Activity
addaleax commentedon Feb 26, 2019
See also the (very recent) discussion in nodejs/node-addon-examples#91.
Is there a reason that
napi_get_value_double+ a manual integer check doesn’t work for you?jorangreef commentedon Feb 27, 2019
Thanks for the pointer to that discussion.
I hadn't thought of
napi_get_value_double().napi_get_value_double()could work for floats, but would it work forNaN?jorangreef commentedon Feb 27, 2019
I am trying to prevent the situation where a user dereferences a JS object property which doesn't exist, while adding it to another integer as part of passing an offset or size value to a native addon.
e.g. Passing an offset of
object.HEADER + 10whereobject.HEADERis a typo and should have beenobject.HEADING.This will end up passing an offset of
NaN, which the native addon will receive as0, which could potentially lead to a misdirected read/write in a native addon, with the user being none the wiser.legendecas commentedon Feb 27, 2019
AFAIK, there is an NAN defined in
math.h.napi_get_value_doublewould return theNANif the js value isNaN.addaleax commentedon Feb 27, 2019
I think
static_cast<double>(static_cast<int64_t>(double_value)) == double_valuewould be a simple and valid check that correctly accounts forInfinity/-InfinityandNaN.It doesn’t work for integers larger than ±2**63, but chances are you’re interested in checking whether the value is smaller than that anyway? You may want to adjust
int64_tto the actual integer type you’ll end up using.jorangreef commentedon Feb 27, 2019
Thanks @legendecas and @addaleax for the tips!
Would
(double)((int64_t)(double_value)) == double_valuebe completely safe in C?jorangreef commentedon Feb 27, 2019
In other words, would this not lead to undefined behavior if the double represented NaN or Infinity?
addaleax commentedon Feb 27, 2019
@jorangreef I think the result of the cast-to-integer is undefined in those cases, but that doesn’t matter because whatever the result, it is an integer and so it won’t compare equal to the original value.
Casting to integer and then to double and seeing if the result is the same is also what V8 does when we call
Number::New()and it has to decide whether to represent the result as an integer or not.