From 32bcf4ca27bba9d4e48418f12dc6d7c2252e71ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Tue, 1 Aug 2023 22:36:12 +0000
Subject: [PATCH] permission: improve path traversal protection

Always use the original implementation of pathModule.resolve. If the
application overwrites the value of pathModule.resolve with a custom
implementation, it should not have any effect on the permission model.

PR-URL: https://github.com/nodejs-private/node-private/pull/456
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
CVE-ID: CVE-2023-39331
---
 lib/internal/fs/utils.js                 |  5 +++--
 test/fixtures/permission/fs-traversal.js | 14 +++++++++-----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
index f84133296e8..0fc9bd9b987 100644
--- a/lib/internal/fs/utils.js
+++ b/lib/internal/fs/utils.js
@@ -708,12 +708,13 @@ 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;
 function possiblyTransformPath(path) {
   if (permission.isEnabled()) {
     if (typeof path === 'string') {
-      return pathModule.resolve(path);
+      return resolvePath(path);
     } else if (Buffer.isBuffer(path)) {
-      return Buffer.from(pathModule.resolve(path.toString()));
+      return Buffer.from(resolvePath(path.toString()));
     }
   }
   return path;
diff --git a/test/fixtures/permission/fs-traversal.js b/test/fixtures/permission/fs-traversal.js
index 2c35fb90ed6..e45600fff88 100644
--- a/test/fixtures/permission/fs-traversal.js
+++ b/test/fixtures/permission/fs-traversal.js
@@ -6,6 +6,10 @@ const assert = require('assert');
 const fs = require('fs');
 const path = require('path');
 
+// This should not affect how the permission model resolves paths.
+const { resolve } = path;
+path.resolve = (s) => s;
+
 const blockedFolder = process.env.BLOCKEDFOLDER;
 const allowedFolder = process.env.ALLOWEDFOLDER;
 const traversalPath = allowedFolder + '../file.md';
@@ -27,7 +31,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemWrite',
-    resource: path.toNamespacedPath(path.resolve(traversalPath)),
+    resource: path.toNamespacedPath(resolve(traversalPath)),
   }));
 }
 
@@ -39,7 +43,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
-    resource: path.toNamespacedPath(path.resolve(traversalPath)),
+    resource: path.toNamespacedPath(resolve(traversalPath)),
   }));
 }
 
@@ -51,7 +55,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemWrite',
-    resource: path.resolve(traversalFolderPath + 'XXXXXX'),
+    resource: resolve(traversalFolderPath + 'XXXXXX'),
   }));
 }
 
@@ -63,7 +67,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemWrite',
-    resource: path.resolve(traversalFolderPath + 'XXXXXX'),
+    resource: resolve(traversalFolderPath + 'XXXXXX'),
   }));
 }
 
@@ -75,7 +79,7 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
-    resource: path.resolve(traversalPath),
+    resource: resolve(traversalPath),
   }));
 }