Skip to content

Commit 99f2173

Browse files
tniessenRafaelGSS
authored andcommittedJan 15, 2025
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 a9980eb commit 99f2173

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,
@@ -420,6 +421,23 @@ const win32 = {
420421
if (tail.length > 0 &&
421422
isPathSeparator(StringPrototypeCharCodeAt(path, len - 1)))
422423
tail += '\\';
424+
if (!isAbsolute && device === undefined && StringPrototypeIncludes(path, ':')) {
425+
// If the original path was not absolute and if we have not been able to
426+
// resolve it relative to a particular device, we need to ensure that the
427+
// `tail` has not become something that Windows might interpret as an
428+
// absolute path. See CVE-2024-36139.
429+
if (tail.length >= 2 &&
430+
isWindowsDeviceRoot(StringPrototypeCharCodeAt(tail, 0)) &&
431+
StringPrototypeCharCodeAt(tail, 1) === CHAR_COLON) {
432+
return `.\\${tail}`;
433+
}
434+
let index = StringPrototypeIndexOf(path, ':');
435+
do {
436+
if (index === len - 1 || isPathSeparator(StringPrototypeCharCodeAt(path, index + 1))) {
437+
return `.\\${tail}`;
438+
}
439+
} while ((index = StringPrototypeIndexOf(path, ':', index + 1)) !== -1);
440+
}
423441
if (device === undefined) {
424442
return isAbsolute ? `\\${tail}` : tail;
425443
}

‎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)
Please sign in to comment.