diff --git a/lib/fs.js b/lib/fs.js index 0aa03ffe229140..ecb61aefa6aa31 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -453,7 +453,12 @@ function read(fd, buffer, offset, length, position, callback) { validateBuffer(buffer); callback = maybeCallback(callback); - offset |= 0; + if (offset == null) { + offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } + length |= 0; if (length === 0) { @@ -490,7 +495,12 @@ function readSync(fd, buffer, offset, length, position) { validateInt32(fd, 'fd', 0); validateBuffer(buffer); - offset |= 0; + if (offset == null) { + offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } + length |= 0; if (length === 0) { diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index c8203055b57cfe..67237c954babc1 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -206,7 +206,12 @@ async function read(handle, buffer, offset, length, position) { validateFileHandle(handle); validateBuffer(buffer); - offset |= 0; + if (offset == null) { + offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } + length |= 0; if (length === 0) diff --git a/src/node_file.cc b/src/node_file.cc index 138848c49da45e..5bd0237e7b0556 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1851,7 +1851,7 @@ static void WriteString(const FunctionCallbackInfo& args) { * * 0 fd int32. file descriptor * 1 buffer instance of Buffer - * 2 offset int32. offset to start reading into inside buffer + * 2 offset int64. offset to start reading into inside buffer * 3 length int32. length to read * 4 position int64. file position - -1 for current position */ @@ -1869,15 +1869,17 @@ static void Read(const FunctionCallbackInfo& args) { char* buffer_data = Buffer::Data(buffer_obj); size_t buffer_length = Buffer::Length(buffer_obj); - CHECK(args[2]->IsInt32()); - const size_t off = static_cast(args[2].As()->Value()); - CHECK_LT(off, buffer_length); + CHECK(IsSafeJsInt(args[2])); + const int64_t off_64 = args[2].As()->Value(); + CHECK_GE(off_64, 0); + CHECK_LT(static_cast(off_64), buffer_length); + const size_t off = static_cast(off_64); CHECK(args[3]->IsInt32()); const size_t len = static_cast(args[3].As()->Value()); CHECK(Buffer::IsWithinBounds(off, len, buffer_length)); - CHECK(args[4]->IsNumber()); + CHECK(IsSafeJsInt(args[4])); const int64_t pos = args[4].As()->Value(); char* buf = buffer_data + off; diff --git a/src/util-inl.h b/src/util-inl.h index 5f8e7acb2a8e52..c06a0ae84c88f5 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -24,6 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include #include #include "util.h" @@ -521,6 +522,17 @@ void ArrayBufferViewContents::Read(v8::Local abv) { } } +// ECMA262 20.1.2.5 +inline bool IsSafeJsInt(v8::Local v) { + if (!v->IsNumber()) return false; + double v_d = v.As()->Value(); + if (std::isnan(v_d)) return false; + if (std::isinf(v_d)) return false; + if (std::trunc(v_d) != v_d) return false; // not int + if (std::abs(v_d) <= static_cast(kMaxSafeJsInteger)) return true; + return false; +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/util.h b/src/util.h index a94e88f232fd31..c8bb44721f6250 100644 --- a/src/util.h +++ b/src/util.h @@ -184,6 +184,11 @@ void DumpBacktrace(FILE* fp); #define UNREACHABLE(...) \ ERROR_AND_ABORT("Unreachable code reached" __VA_OPT__(": ") __VA_ARGS__) +// ECMA262 20.1.2.6 Number.MAX_SAFE_INTEGER (2^53-1) +constexpr int64_t kMaxSafeJsInteger = 9007199254740991; + +inline bool IsSafeJsInt(v8::Local v); + // TAILQ-style intrusive list node. template class ListNode; diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js index b51df515898231..f5ac78a23026fd 100644 --- a/test/parallel/test-fs-read-type.js +++ b/test/parallel/test-fs-read-type.js @@ -50,6 +50,20 @@ assert.throws(() => { 'Received -1' }); +assert.throws(() => { + fs.read(fd, + Buffer.allocUnsafe(expected.length), + NaN, + expected.length, + 0, + common.mustNotCall()); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "offset" is out of range. It must be an integer. ' + + 'Received NaN' +}); + assert.throws(() => { fs.read(fd, Buffer.allocUnsafe(expected.length), @@ -103,6 +117,19 @@ assert.throws(() => { 'It must be >= 0 && <= 4. Received -1' }); +assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + NaN, + expected.length, + 0); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "offset" is out of range. It must be an integer. ' + + 'Received NaN' +}); + assert.throws(() => { fs.readSync(fd, Buffer.allocUnsafe(expected.length),