Skip to content

Commit 0afc6f9

Browse files
RafaelGSStniessen
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. Co-Authored-By: Tobias Nießen <tobias.niessen@tuwien.ac.at> PR-URL: nodejs-private/node-private#555 Backport-PR-URL: nodejs-private/node-private#665 CVE-ID: CVE-2025-23084
1 parent f2ad4d3 commit 0afc6f9

File tree

3 files changed

+51
-0
lines changed

3 files changed

+51
-0
lines changed

lib/path.js

Lines changed: 18 additions & 0 deletions
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,
@@ -414,6 +415,23 @@ const win32 = {
414415
if (tail.length > 0 &&
415416
isPathSeparator(StringPrototypeCharCodeAt(path, len - 1)))
416417
tail += '\\';
418+
if (!isAbsolute && device === undefined && StringPrototypeIncludes(path, ':')) {
419+
// If the original path was not absolute and if we have not been able to
420+
// resolve it relative to a particular device, we need to ensure that the
421+
// `tail` has not become something that Windows might interpret as an
422+
// absolute path. See CVE-2024-36139.
423+
if (tail.length >= 2 &&
424+
isWindowsDeviceRoot(StringPrototypeCharCodeAt(tail, 0)) &&
425+
StringPrototypeCharCodeAt(tail, 1) === CHAR_COLON) {
426+
return `.\\${tail}`;
427+
}
428+
let index = StringPrototypeIndexOf(path, ':');
429+
do {
430+
if (index === len - 1 || isPathSeparator(StringPrototypeCharCodeAt(path, index + 1))) {
431+
return `.\\${tail}`;
432+
}
433+
} while ((index = StringPrototypeIndexOf(path, ':', index + 1)) !== -1);
434+
}
417435
if (device === undefined) {
418436
return isAbsolute ? `\\${tail}` : tail;
419437
}

test/parallel/test-path-join.js

Lines changed: 7 additions & 0 deletions
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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,32 @@ assert.strictEqual(path.win32.normalize('foo/bar\\baz'), 'foo\\bar\\baz');
4343
assert.strictEqual(path.win32.normalize('\\\\.\\foo'), '\\\\.\\foo');
4444
assert.strictEqual(path.win32.normalize('\\\\.\\foo\\'), '\\\\.\\foo\\');
4545

46+
// Tests related to CVE-2024-36139. Path traversal should not result in changing
47+
// the root directory on Windows.
48+
assert.strictEqual(path.win32.normalize('test/../C:/Windows'), '.\\C:\\Windows');
49+
assert.strictEqual(path.win32.normalize('test/../C:Windows'), '.\\C:Windows');
50+
assert.strictEqual(path.win32.normalize('./upload/../C:/Windows'), '.\\C:\\Windows');
51+
assert.strictEqual(path.win32.normalize('./upload/../C:x'), '.\\C:x');
52+
assert.strictEqual(path.win32.normalize('test/../??/D:/Test'), '.\\??\\D:\\Test');
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:\\'), '.\\F:\\');
56+
assert.strictEqual(path.win32.normalize('test/C:foo/../../F:\\'), '.\\F:\\');
57+
assert.strictEqual(path.win32.normalize('test/C:/../../F:x'), '.\\F:x');
58+
assert.strictEqual(path.win32.normalize('test/C:foo/../../F:x'), '.\\F:x');
59+
assert.strictEqual(path.win32.normalize('/test/../??/D:/Test'), '\\??\\D:\\Test');
60+
assert.strictEqual(path.win32.normalize('/test/../?/D:/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'), '\\\\?\\?\\D:\\Test');
64+
assert.strictEqual(path.win32.normalize('\\\\?\\test/../../?/D:/Test'), '\\\\?\\?\\D:\\Test');
65+
assert.strictEqual(path.win32.normalize('\\\\.\\test/../?/D:/Test'), '\\\\.\\?\\D:\\Test');
66+
assert.strictEqual(path.win32.normalize('\\\\.\\test/../../?/D:/Test'), '\\\\.\\?\\D:\\Test');
67+
assert.strictEqual(path.win32.normalize('//server/share/dir/../../../?/D:/file'),
68+
'\\\\server\\share\\?\\D:\\file');
69+
assert.strictEqual(path.win32.normalize('//server/goodshare/../badshare/file'),
70+
'\\\\server\\goodshare\\badshare\\file');
71+
4672
assert.strictEqual(path.posix.normalize('./fixtures///b/../b/c.js'),
4773
'fixtures/b/c.js');
4874
assert.strictEqual(path.posix.normalize('/foo/../../../bar'), '/bar');

0 commit comments

Comments
 (0)