Skip to content

Commit

Permalink
src: fix validation of negative offset to avoid abort
Browse files Browse the repository at this point in the history
Fixes: #24640
Signed-off-by: James M Snell <[email protected]>

PR-URL: #38421
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
  • Loading branch information
jasnell authored and targos committed Jun 5, 2021
1 parent f1366ad commit 8098d59
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 10 deletions.
8 changes: 4 additions & 4 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
);

Expand Down
4 changes: 0 additions & 4 deletions test/parallel/test-fs-read-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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(() => {
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-fs-write-negativeoffset.js
Original file line number Diff line number Diff line change
@@ -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);
}));

0 comments on commit 8098d59

Please sign in to comment.