From 0afc6f960017708df3870ff1d61249443873637b Mon Sep 17 00:00:00 2001
From: RafaelGSS <rafael.nunu@hotmail.com>
Date: Mon, 20 Jan 2025 11:40:16 -0300
Subject: [PATCH] path: fix path traversal in normalize() on Windows
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Without this patch, on Windows, normalizing a relative path might result
in a path that Windows considers absolute. In rare cases, this might
lead to path traversal vulnerabilities in user code.

We attempt to detect those cases and return a relative path instead.

Co-Authored-By: Tobias Nießen <tobias.niessen@tuwien.ac.at>
PR-URL: https://github.com/nodejs-private/node-private/pull/555
Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/665
CVE-ID: CVE-2025-23084
---
 lib/path.js                          | 18 ++++++++++++++++++
 test/parallel/test-path-join.js      |  7 +++++++
 test/parallel/test-path-normalize.js | 26 ++++++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/lib/path.js b/lib/path.js
index 8b9d6d0b6b9621..1a59a66f66bb2a 100644
--- a/lib/path.js
+++ b/lib/path.js
@@ -26,6 +26,7 @@ const {
   ArrayPrototypeSlice,
   FunctionPrototypeBind,
   StringPrototypeCharCodeAt,
+  StringPrototypeIncludes,
   StringPrototypeIndexOf,
   StringPrototypeLastIndexOf,
   StringPrototypeRepeat,
@@ -414,6 +415,23 @@ const win32 = {
     if (tail.length > 0 &&
         isPathSeparator(StringPrototypeCharCodeAt(path, len - 1)))
       tail += '\\';
+    if (!isAbsolute && device === undefined && StringPrototypeIncludes(path, ':')) {
+      // If the original path was not absolute and if we have not been able to
+      // resolve it relative to a particular device, we need to ensure that the
+      // `tail` has not become something that Windows might interpret as an
+      // absolute path. See CVE-2024-36139.
+      if (tail.length >= 2 &&
+          isWindowsDeviceRoot(StringPrototypeCharCodeAt(tail, 0)) &&
+          StringPrototypeCharCodeAt(tail, 1) === CHAR_COLON) {
+        return `.\\${tail}`;
+      }
+      let index = StringPrototypeIndexOf(path, ':');
+      do {
+        if (index === len - 1 || isPathSeparator(StringPrototypeCharCodeAt(path, index + 1))) {
+          return `.\\${tail}`;
+        }
+      } while ((index = StringPrototypeIndexOf(path, ':', index + 1)) !== -1);
+    }
     if (device === undefined) {
       return isAbsolute ? `\\${tail}` : tail;
     }
diff --git a/test/parallel/test-path-join.js b/test/parallel/test-path-join.js
index d6d18399960d0b..b8d6375989a75e 100644
--- a/test/parallel/test-path-join.js
+++ b/test/parallel/test-path-join.js
@@ -110,6 +110,13 @@ joinTests.push([
       [['c:.', 'file'], 'c:file'],
       [['c:', '/'], 'c:\\'],
       [['c:', 'file'], 'c:\\file'],
+      // Path traversal in previous versions of Node.js.
+      [['./upload', '/../C:/Windows'], '.\\C:\\Windows'],
+      [['upload', '../', 'C:foo'], '.\\C:foo'],
+      [['test/..', '??/D:/Test'], '.\\??\\D:\\Test'],
+      [['test', '..', 'D:'], '.\\D:'],
+      [['test', '..', 'D:\\'], '.\\D:\\'],
+      [['test', '..', 'D:foo'], '.\\D:foo'],
     ]
   ),
 ]);
diff --git a/test/parallel/test-path-normalize.js b/test/parallel/test-path-normalize.js
index 7a3d02cb7fe126..8b537676dbf45d 100644
--- a/test/parallel/test-path-normalize.js
+++ b/test/parallel/test-path-normalize.js
@@ -43,6 +43,32 @@ assert.strictEqual(path.win32.normalize('foo/bar\\baz'), 'foo\\bar\\baz');
 assert.strictEqual(path.win32.normalize('\\\\.\\foo'), '\\\\.\\foo');
 assert.strictEqual(path.win32.normalize('\\\\.\\foo\\'), '\\\\.\\foo\\');
 
+// Tests related to CVE-2024-36139. Path traversal should not result in changing
+// the root directory on Windows.
+assert.strictEqual(path.win32.normalize('test/../C:/Windows'), '.\\C:\\Windows');
+assert.strictEqual(path.win32.normalize('test/../C:Windows'), '.\\C:Windows');
+assert.strictEqual(path.win32.normalize('./upload/../C:/Windows'), '.\\C:\\Windows');
+assert.strictEqual(path.win32.normalize('./upload/../C:x'), '.\\C:x');
+assert.strictEqual(path.win32.normalize('test/../??/D:/Test'), '.\\??\\D:\\Test');
+assert.strictEqual(path.win32.normalize('test/C:/../../F:'), '.\\F:');
+assert.strictEqual(path.win32.normalize('test/C:foo/../../F:'), '.\\F:');
+assert.strictEqual(path.win32.normalize('test/C:/../../F:\\'), '.\\F:\\');
+assert.strictEqual(path.win32.normalize('test/C:foo/../../F:\\'), '.\\F:\\');
+assert.strictEqual(path.win32.normalize('test/C:/../../F:x'), '.\\F:x');
+assert.strictEqual(path.win32.normalize('test/C:foo/../../F:x'), '.\\F:x');
+assert.strictEqual(path.win32.normalize('/test/../??/D:/Test'), '\\??\\D:\\Test');
+assert.strictEqual(path.win32.normalize('/test/../?/D:/Test'), '\\?\\D:\\Test');
+assert.strictEqual(path.win32.normalize('//test/../??/D:/Test'), '\\\\test\\..\\??\\D:\\Test');
+assert.strictEqual(path.win32.normalize('//test/../?/D:/Test'), '\\\\test\\..\\?\\D:\\Test');
+assert.strictEqual(path.win32.normalize('\\\\?\\test/../?/D:/Test'), '\\\\?\\?\\D:\\Test');
+assert.strictEqual(path.win32.normalize('\\\\?\\test/../../?/D:/Test'), '\\\\?\\?\\D:\\Test');
+assert.strictEqual(path.win32.normalize('\\\\.\\test/../?/D:/Test'), '\\\\.\\?\\D:\\Test');
+assert.strictEqual(path.win32.normalize('\\\\.\\test/../../?/D:/Test'), '\\\\.\\?\\D:\\Test');
+assert.strictEqual(path.win32.normalize('//server/share/dir/../../../?/D:/file'),
+                   '\\\\server\\share\\?\\D:\\file');
+assert.strictEqual(path.win32.normalize('//server/goodshare/../badshare/file'),
+                   '\\\\server\\goodshare\\badshare\\file');
+
 assert.strictEqual(path.posix.normalize('./fixtures///b/../b/c.js'),
                    'fixtures/b/c.js');
 assert.strictEqual(path.posix.normalize('/foo/../../../bar'), '/bar');