Skip to content

Commit

Permalink
fs: validate fds as int32s
Browse files Browse the repository at this point in the history
This commit updates the JS layer's validation of file
descriptors to check for int32s >= 0 instead of uint32s.

PR-URL: nodejs#28984
Fixes: nodejs#28980
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
cjihrig authored and Trott committed Aug 7, 2019
1 parent 84a6384 commit c072a80
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 26 deletions.
36 changes: 18 additions & 18 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,14 +395,14 @@ function readFileSync(path, options) {
}

function close(fd, callback) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
binding.close(fd, req);
}

function closeSync(fd) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);

const ctx = {};
binding.close(fd, undefined, ctx);
Expand Down Expand Up @@ -449,7 +449,7 @@ function openSync(path, flags, mode) {
}

function read(fd, buffer, offset, length, position, callback) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
validateBuffer(buffer);
callback = maybeCallback(callback);

Expand Down Expand Up @@ -487,7 +487,7 @@ Object.defineProperty(read, internalUtil.customPromisifyArgs,
{ value: ['bytesRead', 'buffer'], enumerable: false });

function readSync(fd, buffer, offset, length, position) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
validateBuffer(buffer);

offset |= 0;
Expand Down Expand Up @@ -524,7 +524,7 @@ function write(fd, buffer, offset, length, position, callback) {
callback(err, written || 0, buffer);
}

validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);

const req = new FSReqCallback();
req.oncomplete = wrapper;
Expand Down Expand Up @@ -564,7 +564,7 @@ Object.defineProperty(write, internalUtil.customPromisifyArgs,
// OR
// fs.writeSync(fd, string[, position[, encoding]]);
function writeSync(fd, buffer, offset, length, position) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
const ctx = {};
let result;
if (isArrayBufferView(buffer)) {
Expand Down Expand Up @@ -661,7 +661,7 @@ function ftruncate(fd, len = 0, callback) {
callback = len;
len = 0;
}
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
validateInteger(len, 'len');
len = Math.max(0, len);
const req = new FSReqCallback();
Expand All @@ -670,7 +670,7 @@ function ftruncate(fd, len = 0, callback) {
}

function ftruncateSync(fd, len = 0) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
validateInteger(len, 'len');
len = Math.max(0, len);
const ctx = {};
Expand All @@ -694,28 +694,28 @@ function rmdirSync(path) {
}

function fdatasync(fd, callback) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
binding.fdatasync(fd, req);
}

function fdatasyncSync(fd) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
const ctx = {};
binding.fdatasync(fd, undefined, ctx);
handleErrorFromBinding(ctx);
}

function fsync(fd, callback) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
binding.fsync(fd, req);
}

function fsyncSync(fd) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
const ctx = {};
binding.fsync(fd, undefined, ctx);
handleErrorFromBinding(ctx);
Expand Down Expand Up @@ -801,7 +801,7 @@ function fstat(fd, options, callback) {
callback = options;
options = {};
}
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
const req = new FSReqCallback(options.bigint);
req.oncomplete = makeStatsCallback(callback);
binding.fstat(fd, options.bigint, req);
Expand Down Expand Up @@ -832,7 +832,7 @@ function stat(path, options, callback) {
}

function fstatSync(fd, options = {}) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
const ctx = { fd };
const stats = binding.fstat(fd, options.bigint, undefined, ctx);
handleErrorFromBinding(ctx);
Expand Down Expand Up @@ -1065,7 +1065,7 @@ function lchownSync(path, uid, gid) {
}

function fchown(fd, uid, gid, callback) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
validateUint32(uid, 'uid');
validateUint32(gid, 'gid');

Expand All @@ -1075,7 +1075,7 @@ function fchown(fd, uid, gid, callback) {
}

function fchownSync(fd, uid, gid) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
validateUint32(uid, 'uid');
validateUint32(gid, 'gid');

Expand Down Expand Up @@ -1126,7 +1126,7 @@ function utimesSync(path, atime, mtime) {
}

function futimes(fd, atime, mtime, callback) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
atime = toUnixTimestamp(atime, 'atime');
mtime = toUnixTimestamp(mtime, 'mtime');
const req = new FSReqCallback();
Expand All @@ -1135,7 +1135,7 @@ function futimes(fd, atime, mtime, callback) {
}

function futimesSync(fd, atime, mtime) {
validateUint32(fd, 'fd');
validateInt32(fd, 'fd', 0);
atime = toUnixTimestamp(atime, 'atime');
mtime = toUnixTimestamp(mtime, 'mtime');
const ctx = {};
Expand Down
31 changes: 24 additions & 7 deletions test/parallel/test-fs-fchown.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ require('../common');
const assert = require('assert');
const fs = require('fs');

function test(input, errObj) {
function testFd(input, errObj) {
assert.throws(() => fs.fchown(input), errObj);
assert.throws(() => fs.fchownSync(input), errObj);
errObj.message = errObj.message.replace('fd', 'uid');
}

function testUid(input, errObj) {
assert.throws(() => fs.fchown(1, input), errObj);
assert.throws(() => fs.fchownSync(1, input), errObj);
errObj.message = errObj.message.replace('uid', 'gid');
}

function testGid(input, errObj) {
assert.throws(() => fs.fchown(1, 1, input), errObj);
assert.throws(() => fs.fchownSync(1, 1, input), errObj);
}
Expand All @@ -22,7 +26,11 @@ function test(input, errObj) {
message: 'The "fd" argument must be of type number. Received type ' +
typeof input
};
test(input, errObj);
testFd(input, errObj);
errObj.message = errObj.message.replace('fd', 'uid');
testUid(input, errObj);
errObj.message = errObj.message.replace('uid', 'gid');
testGid(input, errObj);
});

[Infinity, NaN].forEach((input) => {
Expand All @@ -32,15 +40,24 @@ function test(input, errObj) {
message: 'The value of "fd" is out of range. It must be an integer. ' +
`Received ${input}`
};
test(input, errObj);
testFd(input, errObj);
errObj.message = errObj.message.replace('fd', 'uid');
testUid(input, errObj);
errObj.message = errObj.message.replace('uid', 'gid');
testGid(input, errObj);
});

[-1, 2 ** 32].forEach((input) => {
const errObj = {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "fd" is out of range. It must be ' +
`>= 0 && < 4294967296. Received ${input}`
`>= 0 && <= 2147483647. Received ${input}`
};
test(input, errObj);
testFd(input, errObj);
errObj.message = 'The value of "uid" is out of range. It must be >= 0 && ' +
`< 4294967296. Received ${input}`;
testUid(input, errObj);
errObj.message = errObj.message.replace('uid', 'gid');
testGid(input, errObj);
});
2 changes: 1 addition & 1 deletion test/parallel/test-fs-utimes.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ const expectRangeError = {
code: 'ERR_OUT_OF_RANGE',
type: RangeError,
message: 'The value of "fd" is out of range. ' +
'It must be >= 0 && < 4294967296. Received -1'
'It must be >= 0 && <= 2147483647. Received -1'
};
// futimes-only error cases
{
Expand Down

0 comments on commit c072a80

Please sign in to comment.