From 378e0e778b0bf7e1d60797e64db42d989a1986e5 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 26 Apr 2021 10:58:41 -0700 Subject: [PATCH] src: fix validation of negative offset to avoid abort Fixes: https://github.com/nodejs/node/issues/24640 Signed-off-by: James M Snell PR-URL: https://github.com/nodejs/node/pull/38421 Reviewed-By: Ruben Bridgewater Reviewed-By: Nitzan Uziely Reviewed-By: Darshan Sen --- lib/fs.js | 8 +-- lib/internal/fs/promises.js | 4 +- lib/internal/fs/utils.js | 4 ++ test/parallel/test-fs-read-type.js | 4 -- test/parallel/test-fs-write-negativeoffset.js | 55 +++++++++++++++++++ 5 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-fs-write-negativeoffset.js diff --git a/lib/fs.js b/lib/fs.js index a19f84b4f404bd..7325a24f080182 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -535,7 +535,7 @@ function read(fd, buffer, offset, length, position, callback) { if (offset == null) { offset = 0; } else { - validateInteger(offset, 'offset'); + validateInteger(offset, 'offset', 0); } length |= 0; @@ -589,7 +589,7 @@ function readSync(fd, buffer, offset, length, position) { if (offset == null) { offset = 0; } else { - validateInteger(offset, 'offset'); + validateInteger(offset, 'offset', 0); } length |= 0; @@ -667,7 +667,7 @@ function write(fd, buffer, offset, length, position, callback) { if (offset == null || typeof offset === 'function') { offset = 0; } else { - validateInteger(offset, 'offset'); + validateInteger(offset, 'offset', 0); } if (typeof length !== 'number') length = buffer.length - offset; @@ -715,7 +715,7 @@ function writeSync(fd, buffer, offset, length, position) { if (offset == null) { offset = 0; } else { - validateInteger(offset, 'offset'); + validateInteger(offset, 'offset', 0); } if (typeof length !== 'number') length = buffer.byteLength - offset; diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 7b2c526b59850e..39418446bd9c15 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -366,7 +366,7 @@ async function read(handle, bufferOrOptions, offset, length, position) { if (offset == null) { offset = 0; } else { - validateInteger(offset, 'offset'); + validateInteger(offset, 'offset', 0); } length |= 0; @@ -409,7 +409,7 @@ async function write(handle, buffer, offset, length, position) { if (offset == null) { offset = 0; } else { - validateInteger(offset, 'offset'); + validateInteger(offset, 'offset', 0); } if (typeof length !== 'number') length = buffer.length - offset; diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index b935c495c6883f..abc12b7b02a4ff 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -620,6 +620,10 @@ const validateOffsetLengthWrite = hideStackFrames( if (length > byteLength - offset) { throw new ERR_OUT_OF_RANGE('length', `<= ${byteLength - offset}`, length); } + + if (length < 0) { + throw new ERR_OUT_OF_RANGE('length', '>= 0', length); + } } ); diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js index 0f9bdbab588661..02298ed1c3523d 100644 --- a/test/parallel/test-fs-read-type.js +++ b/test/parallel/test-fs-read-type.js @@ -44,8 +44,6 @@ assert.throws(() => { }, { code: 'ERR_OUT_OF_RANGE', name: 'RangeError', - message: 'The value of "offset" is out of range. It must be >= 0. ' + - 'Received -1' }); assert.throws(() => { @@ -109,8 +107,6 @@ assert.throws(() => { }, { code: 'ERR_OUT_OF_RANGE', name: 'RangeError', - message: 'The value of "offset" is out of range. ' + - 'It must be >= 0. Received -1' }); assert.throws(() => { diff --git a/test/parallel/test-fs-write-negativeoffset.js b/test/parallel/test-fs-write-negativeoffset.js new file mode 100644 index 00000000000000..b1c6ed9039b7d7 --- /dev/null +++ b/test/parallel/test-fs-write-negativeoffset.js @@ -0,0 +1,55 @@ +'use strict'; + +// Tests that passing a negative offset does not crash the process + +const common = require('../common'); + +const { + join, +} = require('path'); + +const { + closeSync, + open, + write, + writeSync, +} = require('fs'); + +const assert = require('assert'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const filename = join(tmpdir.path, 'test.txt'); + +open(filename, 'w+', common.mustSucceed((fd) => { + assert.throws(() => { + write(fd, Buffer.alloc(0), -1, common.mustNotCall()); + }, { + code: 'ERR_OUT_OF_RANGE', + }); + assert.throws(() => { + writeSync(fd, Buffer.alloc(0), -1); + }, { + code: 'ERR_OUT_OF_RANGE', + }); + closeSync(fd); +})); + +const filename2 = join(tmpdir.path, 'test2.txt'); + +// Make sure negative length's don't cause aborts either + +open(filename2, 'w+', common.mustSucceed((fd) => { + assert.throws(() => { + write(fd, Buffer.alloc(0), 0, -1, common.mustNotCall()); + }, { + code: 'ERR_OUT_OF_RANGE', + }); + assert.throws(() => { + writeSync(fd, Buffer.alloc(0), 0, -1); + }, { + code: 'ERR_OUT_OF_RANGE', + }); + closeSync(fd); +}));