fs: validate fds as int32s

cjihrig · targos · commit f194626ffb97 · 2019-08-19T21:18:55.000+02:00
This commit updates the JS layer's validation of file descriptors to check for int32s >= 0 instead of uint32s. PR-URL: #28984 Fixes: #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>
diff --git 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
@@ -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,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);
 });
diff --git 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
 {

Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,7 @@ const expectRangeError = {`
`210`	`210`	`code: 'ERR_OUT_OF_RANGE',`
`211`	`211`	`type: RangeError,`
`212`	`212`	`message: 'The value of "fd" is out of range. ' +`
`213`		`- 'It must be >= 0 && < 4294967296. Received -1'`
	`213`	`+ 'It must be >= 0 && <= 2147483647. Received -1'`
`214`	`214`	`};`
`215`	`215`	`// futimes-only error cases`
`216`	`216`	`{`