Merge branch 'node-v20.15.1-nsolid-v5.3.1-release' into node-v20.x-ns…

…olid-v5.x

CHANGELOG.md

@@ -35,7 +35,8 @@ release.
 </tr>
 <tr>
   <td valign="top">
-<b><a href="doc/changelogs/CHANGELOG_V20.md#20.15.0">20.15.0</a></b><br/>
+<b><a href="doc/changelogs/CHANGELOG_V20.md#20.15.1">20.15.1</a></b><br/>
+<a href="doc/changelogs/CHANGELOG_V20.md#20.15.0">20.15.0</a><br/>
 <a href="doc/changelogs/CHANGELOG_V20.md#20.14.0">20.14.0</a><br/>
 <a href="doc/changelogs/CHANGELOG_V20.md#20.13.1">20.13.1</a><br/>
 <a href="doc/changelogs/CHANGELOG_V20.md#20.13.0">20.13.0</a><br/>

doc/api/permissions.md

@@ -583,8 +583,6 @@ There are constraints you need to know before using this system:
 
 #### Limitations and Known Issues
 
-* When the permission model is enabled, Node.js may resolve some paths
-  differently than when it is disabled.
 * Symbolic links will be followed even to locations outside of the set of paths
   that access has been granted to. Relative symbolic links may allow access to
   arbitrary files and directories. When starting applications with the

doc/changelogs/CHANGELOG_V20.md

@@ -9,6 +9,7 @@
 </tr>
 <tr>
 <td>
+<a href="#20.15.1">20.15.1</a><br/>
 <a href="#20.15.0">20.15.0</a><br/>
 <a href="#20.14.0">20.14.0</a><br/>
 <a href="#20.13.1">20.13.1</a><br/>
@@ -61,6 +62,29 @@
   * [io.js](CHANGELOG_IOJS.md)
   * [Archive](CHANGELOG_ARCHIVE.md)
 
+<a id="20.15.1"></a>
+
+## 2024-07-08, Version 20.15.1 'Iron' (LTS), @RafaelGSS
+
+This is a security release.
+
+### Notable Changes
+
+* CVE-2024-36138 - Bypass incomplete fix of CVE-2024-27980 (High)
+* CVE-2024-22020 - Bypass network import restriction via data URL (Medium)
+* CVE-2024-22018 - fs.lstat bypasses permission model (Low)
+* CVE-2024-36137 - fs.fchown/fchmod bypasses permission model (Low)
+* CVE-2024-37372 - Permission model improperly processes UNC paths (Low)
+
+### Commits
+
+* \[[`60e184a6e4`](https://github.com/nodejs/node/commit/60e184a6e4)] - **lib,esm**: handle bypass network-import via data: (RafaelGSS) [nodejs-private/node-private#522](https://github.com/nodejs-private/node-private/pull/522)
+* \[[`025cbd6936`](https://github.com/nodejs/node/commit/025cbd6936)] - **lib,permission**: support fs.lstat (RafaelGSS) [nodejs-private/node-private#486](https://github.com/nodejs-private/node-private/pull/486)
+* \[[`d38ea17341`](https://github.com/nodejs/node/commit/d38ea17341)] - **lib,permission**: disable fchmod/fchown when pm enabled (RafaelGSS) [nodejs-private/node-private#584](https://github.com/nodejs-private/node-private/pull/584)
+* \[[`1ba624cd3b`](https://github.com/nodejs/node/commit/1ba624cd3b)] - **src**: handle permissive extension on cmd check (RafaelGSS) [nodejs-private/node-private#596](https://github.com/nodejs-private/node-private/pull/596)
+* \[[`2524d00c3d`](https://github.com/nodejs/node/commit/2524d00c3d)] - **src,permission**: fix UNC path resolution (RafaelGSS) [nodejs-private/node-private#581](https://github.com/nodejs-private/node-private/pull/581)
+* \[[`484cb0f13c`](https://github.com/nodejs/node/commit/484cb0f13c)] - **src,permission**: resolve path on fs\_permission (Rafael Gonzaga) [#52761](https://github.com/nodejs/node/pull/52761)
+
 <a id="20.15.0"></a>
 
 ## 2024-06-20, Version 20.15.0 'Iron' (LTS), @marco-ippolito

doc/changelogs/NSOLID_CHANGELOG_V5_NODE_V20.md

@@ -2,6 +2,12 @@
 
 <!--lint disable maximum-line-length no-literal-urls prohibited-strings-->
 
+## 2024-07-08, Version 20.15.1-nsolid-v5.3.1 'Iron'
+
+### Commits
+
+* \[[`9d83482cae`](https://github.com/nodesource/nsolid/commit/9d83482cae)] - Merge tag 'v20.15.1' into node-v20.15.1-nsolid-v5.3.1-release (Trevor Norris)
+
 ## 2024-06-24, Version 20.15.0-nsolid-v5.3.0 'Iron'
 
 ### Commits

lib/fs.js

@@ -1554,6 +1554,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;
@@ -1630,6 +1634,9 @@ function fstatSync(fd, options = { bigint: false }) {
  */
 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(
     pathModule.toNamespacedPath(path),
     options.bigint,
@@ -1888,6 +1895,11 @@ function fchmod(fd, mode, callback) {
   mode = parseFileMode(mode, 'mode');
   callback = makeCallback(callback);
 
+  if (permission.isEnabled()) {
+    callback(new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.'));
+    return;
+  }
+
   const req = new FSReqCallback();
   req.oncomplete = callback;
   binding.fchmod(fd, mode, req);
@@ -1900,6 +1912,9 @@ function fchmod(fd, mode, callback) {
  * @returns {void}
  */
 function fchmodSync(fd, mode) {
+  if (permission.isEnabled()) {
+    throw new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.');
+  }
   binding.fchmod(
     fd,
     parseFileMode(mode, 'mode'),
@@ -2032,6 +2047,10 @@ function fchown(fd, uid, gid, callback) {
   validateInteger(uid, 'uid', -1, kMaxUserId);
   validateInteger(gid, 'gid', -1, kMaxUserId);
   callback = makeCallback(callback);
+  if (permission.isEnabled()) {
+    callback(new ERR_ACCESS_DENIED('fchown API is disabled when Permission Model is enabled.'));
+    return;
+  }
 
   const req = new FSReqCallback();
   req.oncomplete = callback;
@@ -2048,6 +2067,9 @@ function fchown(fd, uid, gid, callback) {
 function fchownSync(fd, uid, gid) {
   validateInteger(uid, 'uid', -1, kMaxUserId);
   validateInteger(gid, 'gid', -1, kMaxUserId);
+  if (permission.isEnabled()) {
+    throw new ERR_ACCESS_DENIED('fchown API is disabled when Permission Model is enabled.');
+  }
 
   binding.fchown(fd, uid, gid);
 }

lib/internal/errors.js

@@ -1113,7 +1113,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);

lib/internal/fs/utils.js

@@ -22,11 +22,8 @@ const {
   Symbol,
   TypedArrayPrototypeAt,
   TypedArrayPrototypeIncludes,
-  uncurryThis,
 } = primordials;
 
-const permission = require('internal/process/permission');
-
 const { Buffer } = require('buffer');
 const {
   codes: {
@@ -65,8 +62,6 @@ const kType = Symbol('type');
 const kStats = Symbol('stats');
 const assert = require('internal/assert');
 
-const { encodeUtf8String } = internalBinding('encoding_binding');
-
 const {
   fs: {
     F_OK = 0,
@@ -695,31 +690,10 @@ const validatePath = hideStackFrames((path, propName = 'path') => {
   );
 });
 
-// TODO(rafaelgss): implement the path.resolve on C++ side
-// See: https://github.com/nodejs/node/pull/44004#discussion_r930958420
-// The permission model needs the absolute path for the fs_permission
-const resolvePath = pathModule.resolve;
-const { isBuffer: BufferIsBuffer, from: BufferFrom } = Buffer;
-const BufferToString = uncurryThis(Buffer.prototype.toString);
-function possiblyTransformPath(path) {
-  if (permission.isEnabled()) {
-    if (typeof path === 'string') {
-      return resolvePath(path);
-    }
-    assert(isUint8Array(path));
-    if (!BufferIsBuffer(path)) path = BufferFrom(path);
-    // Avoid Buffer.from() and use a C++ binding instead to encode the result
-    // of path.resolve() in order to prevent path traversal attacks that
-    // monkey-patch Buffer internals.
-    return encodeUtf8String(resolvePath(BufferToString(path)));
-  }
-  return path;
-}
-
 const getValidatedPath = hideStackFrames((fileURLOrPath, propName = 'path') => {
   const path = toPathIfFileURL(fileURLOrPath);
   validatePath(path, propName);
-  return possiblyTransformPath(path);
+  return path;
 });
 
 const getValidatedFd = hideStackFrames((fd, propName = 'fd') => {
@@ -952,7 +926,6 @@ module.exports = {
   getValidatedFd,
   getValidatedPath,
   handleErrorFromBinding,
-  possiblyTransformPath,
   preprocessSymlinkDestination,
   realpathCacheKey: Symbol('realpathCacheKey'),
   getStatFsFromBinding,

lib/internal/modules/esm/resolve.js

@@ -1105,9 +1105,18 @@ function defaultResolve(specifier, context = {}) {
     } else {
       parsed = new URL(specifier);
     }
-
     // Avoid accessing the `protocol` property due to the lazy getters.
     protocol = parsed.protocol;
+
+    if (protocol === 'data:' &&
+      parsedParentURL.protocol !== 'file:' &&
+      experimentalNetworkImports) {
+      throw new ERR_NETWORK_IMPORT_DISALLOWED(
+        specifier,
+        parsedParentURL,
+        'import data: from a non file: is not allowed',
+      );
+    }
     if (protocol === 'data:' ||
       (experimentalNetworkImports &&
         (
@@ -1118,7 +1127,10 @@ function defaultResolve(specifier, context = {}) {
     ) {
       return { __proto__: null, url: parsed.href };
     }
-  } catch {
+  } catch (e) {
+    if (e?.code === 'ERR_NETWORK_IMPORT_DISALLOWED') {
+      throw e;
+    }
     // Ignore exception
   }
 

src/node_version.h

@@ -24,7 +24,7 @@
 
 #define NODE_MAJOR_VERSION 20
 #define NODE_MINOR_VERSION 15
-#define NODE_PATCH_VERSION 0
+#define NODE_PATCH_VERSION 1
 
 #define NODE_VERSION_IS_LTS 1
 #define NODE_VERSION_LTS_CODENAME "Iron"
@@ -36,7 +36,7 @@
 #define NSOLID_MINOR_VERSION 3
 #define NSOLID_PATCH_VERSION 1
 
-#define NSOLID_VERSION_IS_RELEASE 0
+#define NSOLID_VERSION_IS_RELEASE 1
 
 #ifndef NODE_STRINGIFY
 #define NODE_STRINGIFY(n) NODE_STRINGIFY_HELPER(n)

src/node_worker.cc

@@ -89,7 +89,7 @@ Worker::Worker(Environment* env,
   // Without this check, to use the permission model with
   // workers (--allow-worker) one would need to pass --allow-inspector as well
   if (env->permission()->is_granted(
-          node::permission::PermissionScope::kInspector)) {
+          env, node::permission::PermissionScope::kInspector)) {
     inspector_parent_handle_ =
         GetInspectorParentHandle(env, thread_id_, url.c_str(), name.c_str());
   }

src/permission/child_process_permission.cc

@@ -15,7 +15,8 @@ void ChildProcessPermission::Apply(Environment* env,
   deny_all_ = true;
 }
 
-bool ChildProcessPermission::is_granted(PermissionScope perm,
+bool ChildProcessPermission::is_granted(Environment* env,
+                                        PermissionScope perm,
                                         const std::string_view& param) const {
   return deny_all_ == false;
 }

src/permission/child_process_permission.h

@@ -15,7 +15,8 @@ class ChildProcessPermission final : public PermissionBase {
   void Apply(Environment* env,
              const std::vector<std::string>& allow,
              PermissionScope scope) override;
-  bool is_granted(PermissionScope perm,
+  bool is_granted(Environment* env,
+                  PermissionScope perm,
                   const std::string_view& param = "") const override;
 
  private:

src/permission/fs_permission.cc

@@ -1,6 +1,7 @@
 #include "fs_permission.h"
 #include "base_object-inl.h"
 #include "debug_utils-inl.h"
+#include "env.h"
 #include "path.h"
 #include "v8.h"
 
@@ -51,21 +52,26 @@ void FreeRecursivelyNode(
 }
 
 bool is_tree_granted(
+    node::Environment* env,
     const node::permission::FSPermission::RadixTree* granted_tree,
     const std::string_view& param) {
+  std::string resolved_param = node::PathResolve(env, {param});
 #ifdef _WIN32
-  // is UNC file path
-  if (param.rfind("\\\\", 0) == 0) {
-    // return lookup with normalized param
-    size_t starting_pos = 4;  // "\\?\"
-    if (param.rfind("\\\\?\\UNC\\") == 0) {
-      starting_pos += 4;  // "UNC\"
-    }
-    auto normalized = param.substr(starting_pos);
-    return granted_tree->Lookup(normalized, true);
+  // Remove leading "\\?\" from UNC path
+  if (resolved_param.substr(0, 4) == "\\\\?\\") {
+    resolved_param.erase(0, 4);
+  }
+
+  // Remove leading "UNC\" from UNC path
+  if (resolved_param.substr(0, 4) == "UNC\\") {
+    resolved_param.erase(0, 4);
+  }
+  // Remove leading "//" from UNC path
+  if (resolved_param.substr(0, 2) == "//") {
+    resolved_param.erase(0, 2);
   }
 #endif
-  return granted_tree->Lookup(param, true);
+  return granted_tree->Lookup(resolved_param, true);
 }
 
 void PrintTree(const node::permission::FSPermission::RadixTree::Node* node,
@@ -146,19 +152,20 @@ void FSPermission::GrantAccess(PermissionScope perm, const std::string& res) {
   }
 }
 
-bool FSPermission::is_granted(PermissionScope perm,
+bool FSPermission::is_granted(Environment* env,
+                              PermissionScope perm,
                               const std::string_view& param = "") const {
   switch (perm) {
     case PermissionScope::kFileSystem:
       return allow_all_in_ && allow_all_out_;
     case PermissionScope::kFileSystemRead:
       return !deny_all_in_ &&
              ((param.empty() && allow_all_in_) || allow_all_in_ ||
-              is_tree_granted(&granted_in_fs_, param));
+              is_tree_granted(env, &granted_in_fs_, param));
     case PermissionScope::kFileSystemWrite:
       return !deny_all_out_ &&
              ((param.empty() && allow_all_out_) || allow_all_out_ ||
-              is_tree_granted(&granted_out_fs_, param));
+              is_tree_granted(env, &granted_out_fs_, param));
     default:
       return false;
   }

src/permission/fs_permission.h

@@ -18,7 +18,8 @@ class FSPermission final : public PermissionBase {
   void Apply(Environment* env,
              const std::vector<std::string>& allow,
              PermissionScope scope) override;
-  bool is_granted(PermissionScope perm,
+  bool is_granted(Environment* env,
+                  PermissionScope perm,
                   const std::string_view& param) const override;
 
   struct RadixTree {

src/permission/inspector_permission.cc

@@ -14,7 +14,8 @@ void InspectorPermission::Apply(Environment* env,
   deny_all_ = true;
 }
 
-bool InspectorPermission::is_granted(PermissionScope perm,
+bool InspectorPermission::is_granted(Environment* env,
+                                     PermissionScope perm,
                                      const std::string_view& param) const {
   return deny_all_ == false;
 }

src/permission/inspector_permission.h

@@ -15,7 +15,8 @@ class InspectorPermission final : public PermissionBase {
   void Apply(Environment* env,
              const std::vector<std::string>& allow,
              PermissionScope scope) override;
-  bool is_granted(PermissionScope perm,
+  bool is_granted(Environment* env,
+                  PermissionScope perm,
                   const std::string_view& param = "") const override;
 
  private:

src/permission/permission.cc

@@ -50,10 +50,10 @@ static void Has(const FunctionCallbackInfo<Value>& args) {
       return;
     }
     return args.GetReturnValue().Set(
-        env->permission()->is_granted(scope, *utf8_arg));
+        env->permission()->is_granted(env, scope, *utf8_arg));
   }
 
-  return args.GetReturnValue().Set(env->permission()->is_granted(scope));
+  return args.GetReturnValue().Set(env->permission()->is_granted(env, scope));
 }
 
 }  // namespace