Skip to content

Commit 42d5821

Browse files
tniessenRafaelGSS
authored andcommitted
path: fix path traversal in normalize() on Windows
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. PR-URL: nodejs-private/node-private#555 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> CVE-ID: CVE-2025-23084
1 parent 97291c2 commit 42d5821

File tree

3 files changed

+51
-0
lines changed

3 files changed

+51
-0
lines changed

lib/path.js

+18
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const {
2626
ArrayPrototypeSlice,
2727
FunctionPrototypeBind,
2828
StringPrototypeCharCodeAt,
29+
StringPrototypeIncludes,
2930
StringPrototypeIndexOf,
3031
StringPrototypeLastIndexOf,
3132
StringPrototypeRepeat,
@@ -421,6 +422,23 @@ const win32 = {
421422
if (tail.length > 0 &&
422423
isPathSeparator(StringPrototypeCharCodeAt(path, len - 1)))
423424
tail += '\\';
425+
if (!isAbsolute && device === undefined && StringPrototypeIncludes(path, ':')) {
426+
// If the original path was not absolute and if we have not been able to
427+
// resolve it relative to a particular device, we need to ensure that the
428+
// `tail` has not become something that Windows might interpret as an
429+
// absolute path. See CVE-2024-36139.
430+
if (tail.length >= 2 &&
431+
isWindowsDeviceRoot(StringPrototypeCharCodeAt(tail, 0)) &&
432+
StringPrototypeCharCodeAt(tail, 1) === CHAR_COLON) {
433+
return `.\\${tail}`;
434+
}
435+
let index = StringPrototypeIndexOf(path, ':');
436+
do {
437+
if (index === len - 1 || isPathSeparator(StringPrototypeCharCodeAt(path, index + 1))) {
438+
return `.\\${tail}`;
439+
}
440+
} while ((index = StringPrototypeIndexOf(path, ':', index + 1)) !== -1);
441+
}
424442
if (device === undefined) {
425443
return isAbsolute ? `\\${tail}` : tail;
426444
}

test/parallel/test-path-join.js

+7
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,13 @@ joinTests.push([
110110
[['c:.', 'file'], 'c:file'],
111111
[['c:', '/'], 'c:\\'],
112112
[['c:', 'file'], 'c:\\file'],
113+
// Path traversal in previous versions of Node.js.
114+
[['./upload', '/../C:/Windows'], '.\\C:\\Windows'],
115+
[['upload', '../', 'C:foo'], '.\\C:foo'],
116+
[['test/..', '??/D:/Test'], '.\\??\\D:\\Test'],
117+
[['test', '..', 'D:'], '.\\D:'],
118+
[['test', '..', 'D:\\'], '.\\D:\\'],
119+
[['test', '..', 'D:foo'], '.\\D:foo'],
113120
]
114121
),
115122
]);

test/parallel/test-path-normalize.js

+26
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,32 @@ assert.strictEqual(
4141
);
4242
assert.strictEqual(path.win32.normalize('foo/bar\\baz'), 'foo\\bar\\baz');
4343

44+
// Tests related to CVE-2024-36139. Path traversal should not result in changing
45+
// the root directory on Windows.
46+
assert.strictEqual(path.win32.normalize('test/../C:/Windows'), '.\\C:\\Windows');
47+
assert.strictEqual(path.win32.normalize('test/../C:Windows'), '.\\C:Windows');
48+
assert.strictEqual(path.win32.normalize('./upload/../C:/Windows'), '.\\C:\\Windows');
49+
assert.strictEqual(path.win32.normalize('./upload/../C:x'), '.\\C:x');
50+
assert.strictEqual(path.win32.normalize('test/../??/D:/Test'), '.\\??\\D:\\Test');
51+
assert.strictEqual(path.win32.normalize('test/C:/../../F:'), '.\\F:');
52+
assert.strictEqual(path.win32.normalize('test/C:foo/../../F:'), '.\\F:');
53+
assert.strictEqual(path.win32.normalize('test/C:/../../F:\\'), '.\\F:\\');
54+
assert.strictEqual(path.win32.normalize('test/C:foo/../../F:\\'), '.\\F:\\');
55+
assert.strictEqual(path.win32.normalize('test/C:/../../F:x'), '.\\F:x');
56+
assert.strictEqual(path.win32.normalize('test/C:foo/../../F:x'), '.\\F:x');
57+
assert.strictEqual(path.win32.normalize('/test/../??/D:/Test'), '\\??\\D:\\Test');
58+
assert.strictEqual(path.win32.normalize('/test/../?/D:/Test'), '\\?\\D:\\Test');
59+
assert.strictEqual(path.win32.normalize('//test/../??/D:/Test'), '\\\\test\\..\\??\\D:\\Test');
60+
assert.strictEqual(path.win32.normalize('//test/../?/D:/Test'), '\\\\test\\..\\?\\D:\\Test');
61+
assert.strictEqual(path.win32.normalize('\\\\?\\test/../?/D:/Test'), '\\\\?\\test\\?\\D:\\Test');
62+
assert.strictEqual(path.win32.normalize('\\\\?\\test/../../?/D:/Test'), '\\\\?\\test\\?\\D:\\Test');
63+
assert.strictEqual(path.win32.normalize('\\\\.\\test/../?/D:/Test'), '\\\\.\\test\\?\\D:\\Test');
64+
assert.strictEqual(path.win32.normalize('\\\\.\\test/../../?/D:/Test'), '\\\\.\\test\\?\\D:\\Test');
65+
assert.strictEqual(path.win32.normalize('//server/share/dir/../../../?/D:/file'),
66+
'\\\\server\\share\\?\\D:\\file');
67+
assert.strictEqual(path.win32.normalize('//server/goodshare/../badshare/file'),
68+
'\\\\server\\goodshare\\badshare\\file');
69+
4470
assert.strictEqual(path.posix.normalize('./fixtures///b/../b/c.js'),
4571
'fixtures/b/c.js');
4672
assert.strictEqual(path.posix.normalize('/foo/../../../bar'), '/bar');

0 commit comments

Comments
 (0)