permission: handle fstatfs and add pm supported list

PR-URL: nodejs-private/node-private#441 CVE-ID: CVE-2023-32005
nodejs · Aug 8, 2023 · bd094d6 · bd094d6
1 parent 1bf3429
commit bd094d6
Show file tree

Hide file tree

Showing 4 changed files with 137 additions and 0 deletions.
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -1252,6 +1252,8 @@ static void StatFs(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue path(env->isolate(), args[0]);
   CHECK_NOT_NULL(*path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemRead, path.ToStringView());
 
   bool use_bigint = args[1]->IsTrue();
   FSReqBase* req_wrap_async = GetReqWrap(args, 2, use_bigint);

diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js
@@ -260,3 +260,22 @@ const regularFile = __filename;
     resource: path.toNamespacedPath(blockedFile),
   }));
 }
+
+// fs.exists
+{
+  // It will return false (without performing IO) when permissions is not met
+  fs.exists(blockedFile, (exists) => {
+    assert.equal(exists, false);
+  });
+}
+
+// fs.statfs
+{
+  assert.throws(() => {
+    fs.statfs(blockedFile, () => {});
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'FileSystemRead',
+    resource: path.toNamespacedPath(blockedFile),
+  }));
+}
diff --git a/test/fixtures/permission/fs-write.js b/test/fixtures/permission/fs-write.js
@@ -379,3 +379,16 @@ const absoluteProtectedFolder = path.resolve(relativeProtectedFolder);
     permission: 'FileSystemWrite',
   });
 }
+
+// fs.unlink
+{
+  assert.throws(() => {
+    fs.unlink(blockedFile, (err) => {
+      assert.ifError(err);
+    });
+  }, {
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'FileSystemWrite',
+    resource: path.toNamespacedPath(blockedFile),
+  });
+}
diff --git a/test/parallel/test-permission-fs-supported.js b/test/parallel/test-permission-fs-supported.js
@@ -0,0 +1,103 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+
+// Most of the times, the function called for async and Sync
+// methods are the same on node_file.cc
+function syncAndAsyncAPI(funcName) {
+  return [funcName, funcName + 'Sync'];
+}
+
+// This tests guarantee whenever a new API under fs module is exposed
+// it must contain a test to the permission model.
+// Otherwise, a vulnerability might be exposed. If you are adding a new
+// fs method, please, make sure to include a test for it on test-permission-fs-*
+// and include to the supportedApis list.
+//
+//
+// This list is synced with
+//   fixtures/permission/fs-read and
+//   fixtures/permission/fs-write
+const supportedApis = [
+  ...syncAndAsyncAPI('appendFile'),
+  ...syncAndAsyncAPI('access'),
+  ...syncAndAsyncAPI('chown'),
+  ...syncAndAsyncAPI('chmod'),
+  ...syncAndAsyncAPI('copyFile'),
+  ...syncAndAsyncAPI('cp'),
+  'createReadStream',
+  'createWriteStream',
+  ...syncAndAsyncAPI('exists'),
+  ...syncAndAsyncAPI('lchown'),
+  ...syncAndAsyncAPI('lchmod'),
+  ...syncAndAsyncAPI('link'),
+  ...syncAndAsyncAPI('lutimes'),
+  ...syncAndAsyncAPI('mkdir'),
+  ...syncAndAsyncAPI('mkdtemp'),
+  ...syncAndAsyncAPI('open'),
+  'openAsBlob',
+  ...syncAndAsyncAPI('mkdtemp'),
+  ...syncAndAsyncAPI('readdir'),
+  ...syncAndAsyncAPI('readFile'),
+  ...syncAndAsyncAPI('readlink'),
+  ...syncAndAsyncAPI('rename'),
+  ...syncAndAsyncAPI('rm'),
+  ...syncAndAsyncAPI('rmdir'),
+  ...syncAndAsyncAPI('stat'),
+  ...syncAndAsyncAPI('statfs'),
+  ...syncAndAsyncAPI('statfs'),
+  ...syncAndAsyncAPI('symlink'),
+  ...syncAndAsyncAPI('truncate'),
+  ...syncAndAsyncAPI('unlink'),
+  ...syncAndAsyncAPI('utimes'),
+  'watch',
+  'watchFile',
+  ...syncAndAsyncAPI('writeFile'),
+  ...syncAndAsyncAPI('opendir'),
+];
+
+// Non functions
+const ignoreList = [
+  'constants',
+  'promises',
+  'X_OK',
+  'W_OK',
+  'R_OK',
+  'F_OK',
+  'Dir',
+  'FileReadStream',
+  'FileWriteStream',
+  '_toUnixTimestamp',
+  'Stats',
+  'ReadStream',
+  'WriteStream',
+  'Dirent',
+  // fs.watch is already blocked
+  'unwatchFile',
+  ...syncAndAsyncAPI('lstat'),
+  ...syncAndAsyncAPI('realpath'),
+  // fd required methods
+  ...syncAndAsyncAPI('close'),
+  ...syncAndAsyncAPI('fchown'),
+  ...syncAndAsyncAPI('fchmod'),
+  ...syncAndAsyncAPI('fdatasync'),
+  ...syncAndAsyncAPI('fstat'),
+  ...syncAndAsyncAPI('fsync'),
+  ...syncAndAsyncAPI('ftruncate'),
+  ...syncAndAsyncAPI('futimes'),
+  ...syncAndAsyncAPI('read'),
+  ...syncAndAsyncAPI('readv'),
+  ...syncAndAsyncAPI('write'),
+  ...syncAndAsyncAPI('writev'),
+];
+
+{
+  const fsList = Object.keys(require('fs'));
+  for (const k of fsList) {
+    if (!supportedApis.includes(k) && !ignoreList.includes(k)) {
+      assert.fail(`fs.${k} was exposed but is neither on the supported list ` +
+      'of the permission model nor on the ignore list.');
+    }
+  }
+}