diff --git a/lib/fs.js b/lib/fs.js index 125279040cae39..d066159a0e2dbc 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -521,8 +521,11 @@ function write(fd, buffer, offset, length, position, callback) { if (isArrayBufferView(buffer)) { callback = maybeCallback(callback || position || length || offset); - if (typeof offset !== 'number') + if (typeof offset !== 'number') { offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } if (typeof length !== 'number') length = buffer.length - offset; if (typeof position !== 'number') @@ -560,8 +563,11 @@ function writeSync(fd, buffer, offset, length, position) { if (isArrayBufferView(buffer)) { if (position === undefined) position = null; - if (typeof offset !== 'number') + if (typeof offset !== 'number') { offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } if (typeof length !== 'number') length = buffer.byteLength - offset; validateOffsetLengthWrite(offset, length, buffer.byteLength); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index ddb6b0c2de34a0..d09ff21f668d77 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -235,8 +235,11 @@ async function write(handle, buffer, offset, length, position) { return { bytesWritten: 0, buffer }; if (isUint8Array(buffer)) { - if (typeof offset !== 'number') + if (typeof offset !== 'number') { offset = 0; + } else { + validateSafeInteger(offset, 'offset'); + } if (typeof length !== 'number') length = buffer.length - offset; if (typeof position !== 'number') diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 5a5ac3bfa263b0..5f7a6fb2969912 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -1,6 +1,6 @@ 'use strict'; -const { Buffer, kMaxLength } = require('buffer'); +const { Buffer } = require('buffer'); const { ERR_FS_INVALID_SYMLINK_TYPE, ERR_INVALID_ARG_TYPE, @@ -426,11 +426,8 @@ function validateOffsetLengthWrite(offset, length, byteLength) { if (offset > byteLength) { err = new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset); - } else { - const max = byteLength > kMaxLength ? kMaxLength : byteLength; - if (length > max - offset) { - err = new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length); - } + } else if (length > byteLength - offset) { + err = new ERR_OUT_OF_RANGE('length', `<= ${byteLength - offset}`, length); } if (err !== undefined) { diff --git a/src/node_file.cc b/src/node_file.cc index 3d84549556b0d7..24affc4f9bb2c7 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -88,7 +88,7 @@ const char* kPathSeparator = "/"; const char* kPathSeparator = "\\/"; #endif -#define GET_OFFSET(a) ((a)->IsNumber() ? (a).As()->Value() : -1) +#define GET_OFFSET(a) (IsSafeJsInt(a) ? (a).As()->Value() : -1) #define TRACE_NAME(name) "fs.sync." #name #define GET_TRACE_ENABLED \ (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED \ @@ -1710,9 +1710,11 @@ static void WriteBuffer(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_LE(off, buffer_length); + CHECK(IsSafeJsInt(args[2])); + const int64_t off_64 = args[2].As()->Value(); + CHECK_GE(off_64, 0); + CHECK_LE(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()); diff --git a/test/parallel/test-fs-util-validateoffsetlengthwrite.js b/test/parallel/test-fs-util-validateoffsetlengthwrite.js index 9aca956bd505bf..f2f80ebdaa577a 100644 --- a/test/parallel/test-fs-util-validateoffsetlengthwrite.js +++ b/test/parallel/test-fs-util-validateoffsetlengthwrite.js @@ -22,22 +22,6 @@ const { kMaxLength } = require('buffer'); ); } -// RangeError when byteLength > kMaxLength, and length > kMaxLength - offset . -{ - const offset = kMaxLength; - const length = 100; - const byteLength = kMaxLength + 1; - common.expectsError( - () => validateOffsetLengthWrite(offset, length, byteLength), - { - code: 'ERR_OUT_OF_RANGE', - type: RangeError, - message: 'The value of "length" is out of range. ' + - `It must be <= ${kMaxLength - offset}. Received ${length}` - } - ); -} - // RangeError when byteLength < kMaxLength, and length > byteLength - offset . { const offset = kMaxLength - 150; diff --git a/test/parallel/test-fs-write-buffer.js b/test/parallel/test-fs-write-buffer.js index 362571f2479c84..596ec4a847fa2f 100644 --- a/test/parallel/test-fs-write-buffer.js +++ b/test/parallel/test-fs-write-buffer.js @@ -148,3 +148,25 @@ tmpdir.refresh(); fs.write(fd, Uint8Array.from(expected), cb); })); } + +// fs.write with invalid offset type +{ + const filename = path.join(tmpdir.path, 'write6.txt'); + fs.open(filename, 'w', 0o644, common.mustCall((err, fd) => { + assert.ifError(err); + + assert.throws(() => { + fs.write(fd, + Buffer.from('abcd'), + 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' + }); + })); +}