lib,permission: support fs.lstat

PR-URL: nodejs-private/node-private#486 Fixes: https://hackerone.com/bugs?subject=nodejs&report_id=2145862 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> CVE-ID: CVE-2024-22018
nodejs · Jul 8, 2024 · b9289a6 · b9289a6
1 parent 39f2070
commit b9289a6
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 6 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -1550,6 +1550,10 @@ function lstat(path, options = { bigint: false }, callback) {
   }
   callback = makeStatsCallback(callback);
   path = getValidatedPath(path);
+  if (permission.isEnabled() && !permission.has('fs.read', path)) {
+    callback(new ERR_ACCESS_DENIED('Access to this API has been restricted', 'FileSystemRead', path));
+    return;
+  }
 
   const req = new FSReqCallback(options.bigint);
   req.oncomplete = callback;
@@ -1622,6 +1626,10 @@ function fstatSync(fd, options = { bigint: false }) {
  * @returns {Stats | undefined}
  */
 function lstatSync(path, options = { bigint: false, throwIfNoEntry: true }) {
+  path = getValidatedPath(path);
+  if (permission.isEnabled() && !permission.has('fs.read', path)) {
+    throw new ERR_ACCESS_DENIED('Access to this API has been restricted', 'FileSystemRead', path);
+  }
   const stats = binding.lstat(
     getValidatedPath(path),
     options.bigint,

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -1112,7 +1112,11 @@ module.exports = {
 // Note: Node.js specific errors must begin with the prefix ERR_
 
 E('ERR_ACCESS_DENIED',
-  'Access to this API has been restricted. Permission: %s',
+  function(msg, permission = '', resource = '') {
+    this.permission = permission;
+    this.resource = resource;
+    return msg;
+  },
   Error);
 E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError);
 E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError);

diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js
@@ -161,23 +161,23 @@ const regularFile = __filename;
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
-    // cpSync calls statSync before reading blockedFile
-    resource: path.toNamespacedPath(blockedFolder),
+    // cpSync calls lstatSync before reading blockedFile
+    resource: blockedFile,
   }));
   assert.throws(() => {
     fs.cpSync(blockedFileURL, path.join(blockedFolder, 'any-other-file'));
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
-    // cpSync calls statSync before reading blockedFile
-    resource: path.toNamespacedPath(blockedFolder),
+    // cpSync calls lstatSync before reading blockedFile
+    resource: blockedFile,
   }));
   assert.throws(() => {
     fs.cpSync(blockedFile, path.join(__dirname, 'any-other-file'));
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
-    resource: path.toNamespacedPath(__dirname),
+    resource: blockedFile,
   }));
 }
 
@@ -396,4 +396,23 @@ const regularFile = __filename;
     permission: 'FileSystemRead',
     resource: blockedFolder,
   }));
+}
+
+// fs.lstat
+{
+  assert.throws(() => {
+    fs.lstatSync(blockedFile);
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+  }));
+  assert.throws(() => {
+    fs.lstatSync(path.join(blockedFolder, 'anyfile'));
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+  }));
+
+  // doesNotThrow
+  fs.lstat(regularFile, (err) => {
+    assert.ifError(err);
+  });
 }