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 Apr 29, 2021
1 parent 6350d35 commit 9c06103
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 @@ -630,7 +630,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 @@ -694,7 +694,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 @@ -806,7 +806,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.byteLength - offset;
Expand Down Expand Up @@ -863,7 +863,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 @@ -417,7 +417,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 @@ -460,7 +460,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.byteLength - 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 @@ -655,6 +655,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 @@ -157,8 +155,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 9c06103

Please sign in to comment.