From fa5dae1944d928748170de65b8d955a5b6b2f2d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Tue, 10 Oct 2023 16:30:07 +0000
Subject: [PATCH] permission: fix Uint8Array path traversal

Previous security patches addressed path traversal vulnerabilities for
string and Buffer inputs, but ignored Uint8Array inputs. This commit
fixes the existing logic to account for the latter.

The previous implementation would silently ignore unexpected inputs,
whereas this commit introduces an explicit assertion to prevent that
unsafe behavior.

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

diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
index cab50dd725024a..c7ec755a306299 100644
--- a/lib/internal/fs/utils.js
+++ b/lib/internal/fs/utils.js
@@ -22,6 +22,7 @@ const {
   Symbol,
   TypedArrayPrototypeAt,
   TypedArrayPrototypeIncludes,
+  uncurryThis,
 } = primordials;
 
 const permission = require('internal/process/permission');
@@ -709,13 +710,16 @@ const validatePath = hideStackFrames((path, propName = 'path') => {
 // 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);
-    } else if (Buffer.isBuffer(path)) {
-      return Buffer.from(resolvePath(path.toString()));
     }
+    assert(isUint8Array(path));
+    if (!BufferIsBuffer(path)) path = BufferFrom(path);
+    return BufferFrom(resolvePath(BufferToString(path)));
   }
   return path;
 }
diff --git a/test/fixtures/permission/fs-traversal.js b/test/fixtures/permission/fs-traversal.js
index e45600fff88af7..18243edfeadf7e 100644
--- a/test/fixtures/permission/fs-traversal.js
+++ b/test/fixtures/permission/fs-traversal.js
@@ -15,6 +15,7 @@ const allowedFolder = process.env.ALLOWEDFOLDER;
 const traversalPath = allowedFolder + '../file.md';
 const traversalFolderPath = allowedFolder + '../folder';
 const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
+const uint8ArrayTraversalPath = new TextEncoder().encode(traversalPath);
 
 {
   assert.ok(process.permission.has('fs.read', allowedFolder));
@@ -83,6 +84,18 @@ const bufferTraversalPath = Buffer.from(allowedFolder + '../file.md');
   }));
 }
 
+{
+  assert.throws(() => {
+    fs.readFile(uint8ArrayTraversalPath, (error) => {
+      assert.ifError(error);
+    });
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'FileSystemRead',
+    resource: resolve(traversalPath),
+  }));
+}
+
 {
   assert.ok(!process.permission.has('fs.read', traversalPath));
   assert.ok(!process.permission.has('fs.write', traversalPath));