Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: fix validation of negative offset to avoid abort #38421

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/fs.js
Expand Up @@ -543,7 +543,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 @@ -599,7 +599,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 @@ -679,7 +679,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 @@ -730,7 +730,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
Expand Up @@ -432,7 +432,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 @@ -475,7 +475,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
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
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
@@ -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);
}));