From f194626ffb9794c9339518c59c789e4c4fd7dc2a Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 5 Aug 2019 09:59:28 -0700 Subject: [PATCH] fs: validate fds as int32s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit updates the JS layer's validation of file descriptors to check for int32s >= 0 instead of uint32s. PR-URL: https://github.com/nodejs/node/pull/28984 Fixes: https://github.com/nodejs/node/issues/28980 Reviewed-By: Michaël Zasso Reviewed-By: Gus Caplan Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Rich Trott --- lib/fs.js | 36 ++++++++++++++++----------------- test/parallel/test-fs-fchown.js | 31 +++++++++++++++++++++------- test/parallel/test-fs-utimes.js | 2 +- 3 files changed, 43 insertions(+), 26 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index e890d0c1305b95..baba33a6a2fa8b 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -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); @@ -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); @@ -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; @@ -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; @@ -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)) { @@ -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(); @@ -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 = {}; @@ -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); @@ -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); @@ -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); @@ -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'); @@ -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'); @@ -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(); @@ -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 = {}; diff --git a/test/parallel/test-fs-fchown.js b/test/parallel/test-fs-fchown.js index 832dd071ea172e..4872e550dba37f 100644 --- a/test/parallel/test-fs-fchown.js +++ b/test/parallel/test-fs-fchown.js @@ -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); } @@ -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) => { @@ -32,7 +40,11 @@ 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) => { @@ -40,7 +52,12 @@ function test(input, 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); }); diff --git a/test/parallel/test-fs-utimes.js b/test/parallel/test-fs-utimes.js index e34953423bd11f..e7a06c3661e84c 100644 --- a/test/parallel/test-fs-utimes.js +++ b/test/parallel/test-fs-utimes.js @@ -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 {