fs: fix valid id range on chown, lchown, fchown

PR-URL: #31694 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
nodejs · Mar 9, 2020 · 0a539dd · 0a539dd
1 parent 9dbe6ab
commit 0a539dd
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 17 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -109,9 +109,10 @@ const {
   parseFileMode,
   validateBuffer,
   validateInteger,
-  validateInt32,
-  validateUint32
+  validateInt32
 } = require('internal/validators');
+// 2 ** 32 - 1
+const kMaxUserId = 4294967295;
 
 let truncateWarn = true;
 let fs;
@@ -1153,26 +1154,26 @@ function chmodSync(path, mode) {
 function lchown(path, uid, gid, callback) {
   callback = makeCallback(callback);
   path = getValidatedPath(path);
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
   const req = new FSReqCallback();
   req.oncomplete = callback;
   binding.lchown(pathModule.toNamespacedPath(path), uid, gid, req);
 }
 
 function lchownSync(path, uid, gid) {
   path = getValidatedPath(path);
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
   const ctx = { path };
   binding.lchown(pathModule.toNamespacedPath(path), uid, gid, undefined, ctx);
   handleErrorFromBinding(ctx);
 }
 
 function fchown(fd, uid, gid, callback) {
   validateInt32(fd, 'fd', 0);
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
 
   const req = new FSReqCallback();
   req.oncomplete = makeCallback(callback);
@@ -1181,8 +1182,8 @@ function fchown(fd, uid, gid, callback) {
 
 function fchownSync(fd, uid, gid) {
   validateInt32(fd, 'fd', 0);
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
 
   const ctx = {};
   binding.fchown(fd, uid, gid, undefined, ctx);
@@ -1192,8 +1193,8 @@ function fchownSync(fd, uid, gid) {
 function chown(path, uid, gid, callback) {
   callback = makeCallback(callback);
   path = getValidatedPath(path);
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
 
   const req = new FSReqCallback();
   req.oncomplete = callback;
@@ -1202,8 +1203,8 @@ function chown(path, uid, gid, callback) {
 
 function chownSync(path, uid, gid) {
   path = getValidatedPath(path);
-  validateUint32(uid, 'uid');
-  validateUint32(gid, 'gid');
+  validateInteger(uid, 'uid', -1, kMaxUserId);
+  validateInteger(gid, 'gid', -1, kMaxUserId);
   const ctx = { path };
   binding.chown(pathModule.toNamespacedPath(path), uid, gid, undefined, ctx);
   handleErrorFromBinding(ctx);

diff --git a/test/parallel/test-fs-fchown.js b/test/parallel/test-fs-fchown.js
@@ -44,16 +44,16 @@ function testGid(input, errObj) {
   testGid(input, errObj);
 });
 
-[-1, 2 ** 32].forEach((input) => {
+[-2, 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 && <= 2147483647. Received ${input}`
   };
   testFd(input, errObj);
-  errObj.message = 'The value of "uid" is out of range. It must be >= 0 && ' +
-    `< 4294967296. Received ${input}`;
+  errObj.message = 'The value of "uid" is out of range. It must be >= -1 && ' +
+    `<= 4294967295. Received ${input}`;
   testUid(input, errObj);
   errObj.message = errObj.message.replace('uid', 'gid');
   testGid(input, errObj);