From 81f7a67e329241c80fbbafcc32eafbae17cedad8 Mon Sep 17 00:00:00 2001
From: himself65 <himself65@outlook.com>
Date: Sun, 9 Feb 2020 23:13:41 +0800
Subject: [PATCH] fs: fix valid id range on chown, lchown, fchown

PR-URL: https://github.com/nodejs/node/pull/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>
---
 lib/fs.js                       | 29 +++++++++++++++--------------
 test/parallel/test-fs-fchown.js |  6 +++---
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/lib/fs.js b/lib/fs.js
index 167ee9d7b3fe7f..31fd05dd3ab435 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -102,9 +102,10 @@ const {
   parseMode,
   validateBuffer,
   validateInteger,
-  validateInt32,
-  validateUint32
+  validateInt32
 } = require('internal/validators');
+// 2 ** 32 - 1
+const kMaxUserId = 4294967295;
 
 let truncateWarn = true;
 let fs;
@@ -1124,8 +1125,8 @@ 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);
@@ -1133,8 +1134,8 @@ function lchown(path, uid, gid, callback) {
 
 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);
@@ -1142,8 +1143,8 @@ function lchownSync(path, uid, gid) {
 
 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);
@@ -1152,8 +1153,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);
@@ -1163,8 +1164,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;
@@ -1173,8 +1174,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
index cf28d54e9d9679..03a9ef3780ae6b 100644
--- a/test/parallel/test-fs-fchown.js
+++ b/test/parallel/test-fs-fchown.js
@@ -44,7 +44,7 @@ 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',
@@ -52,8 +52,8 @@ function testGid(input, errObj) {
              `>= 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);