From 46ce27807970157ab1052f70824f46e70e267b5e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Sat, 21 Oct 2023 11:18:54 +0000
Subject: [PATCH] fs: protect against modified Buffer internals in
 possiblyTransformPath

Use encodeUtf8String from the encoding_binding internal binding to
convert the result of path.resolve() to a Uint8Array instead of using
Buffer.from(), whose result can be manipulated by the user by
monkey-patching internals such as Buffer.prototype.utf8Write.

HackerOne report: https://hackerone.com/reports/2218653

PR-URL: https://github.com/nodejs-private/node-private/pull/497
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
CVE-ID: CVE-2024-21896
---
 lib/internal/fs/utils.js                 |  7 ++++-
 test/fixtures/permission/fs-traversal.js | 33 ++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
index e3e1c25f95ea5a..a012ad0a75338e 100644
--- a/lib/internal/fs/utils.js
+++ b/lib/internal/fs/utils.js
@@ -67,6 +67,8 @@ const kType = Symbol('type');
 const kStats = Symbol('stats');
 const assert = require('internal/assert');
 
+const { encodeUtf8String } = internalBinding('encoding_binding');
+
 const {
   fs: {
     F_OK = 0,
@@ -754,7 +756,10 @@ function possiblyTransformPath(path) {
     }
     assert(isUint8Array(path));
     if (!BufferIsBuffer(path)) path = BufferFrom(path);
-    return BufferFrom(resolvePath(BufferToString(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;
 }
diff --git a/test/fixtures/permission/fs-traversal.js b/test/fixtures/permission/fs-traversal.js
index 87132177643c45..f0a14805aa9c71 100644
--- a/test/fixtures/permission/fs-traversal.js
+++ b/test/fixtures/permission/fs-traversal.js
@@ -96,6 +96,39 @@ const uint8ArrayTraversalPath = new TextEncoder().encode(traversalPath);
   }));
 }
 
+// Monkey-patching Buffer internals should also not allow path traversal.
+{
+  const extraChars = '.'.repeat(40);
+  const traversalPathWithExtraChars = traversalPath + extraChars;
+  const traversalPathWithExtraBytes = Buffer.from(traversalPathWithExtraChars);
+
+  Buffer.prototype.utf8Write = ((w) => function(str, ...args) {
+    assert.strictEqual(str, resolve(traversalPath) + extraChars);
+    return w.apply(this, [traversalPath, ...args]);
+  })(Buffer.prototype.utf8Write);
+
+  // Sanity check (remove if the internals of Buffer.from change):
+  // The custom implementation of utf8Write should cause Buffer.from() to encode
+  // traversalPath instead of the sanitized output of resolve().
+  assert.strictEqual(Buffer.from(resolve(traversalPathWithExtraChars)).toString(), traversalPath);
+
+  assert.throws(() => {
+    fs.readFile(traversalPathWithExtraBytes, common.mustNotCall());
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'FileSystemRead',
+    resource: resolve(traversalPathWithExtraChars),
+  }));
+
+  assert.throws(() => {
+    fs.readFile(new TextEncoder().encode(traversalPathWithExtraBytes.toString()), common.mustNotCall());
+  }, common.expectsError({
+    code: 'ERR_ACCESS_DENIED',
+    permission: 'FileSystemRead',
+    resource: resolve(traversalPathWithExtraChars),
+  }));
+}
+
 {
   assert.ok(!process.permission.has('fs.read', traversalPath));
   assert.ok(!process.permission.has('fs.write', traversalPath));