Skip to content

Commit da2d177

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 <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> CVE-ID: CVE-2025-23084
1 parent 14b6317 commit da2d177

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
@@ -24,6 +24,7 @@
2424
const {
2525
FunctionPrototypeBind,
2626
StringPrototypeCharCodeAt,
27+
StringPrototypeIncludes,
2728
StringPrototypeIndexOf,
2829
StringPrototypeLastIndexOf,
2930
StringPrototypeReplace,
@@ -389,6 +390,23 @@ const win32 = {
389390
if (tail.length > 0 &&
390391
isPathSeparator(StringPrototypeCharCodeAt(path, len - 1)))
391392
tail += '\\';
393+
if (!isAbsolute && device === undefined && StringPrototypeIncludes(path, ':')) {
394+
// If the original path was not absolute and if we have not been able to
395+
// resolve it relative to a particular device, we need to ensure that the
396+
// `tail` has not become something that Windows might interpret as an
397+
// absolute path. See CVE-2024-36139.
398+
if (tail.length >= 2 &&
399+
isWindowsDeviceRoot(StringPrototypeCharCodeAt(tail, 0)) &&
400+
StringPrototypeCharCodeAt(tail, 1) === CHAR_COLON) {
401+
return `.\\${tail}`;
402+
}
403+
let index = StringPrototypeIndexOf(path, ':');
404+
do {
405+
if (index === len - 1 || isPathSeparator(StringPrototypeCharCodeAt(path, index + 1))) {
406+
return `.\\${tail}`;
407+
}
408+
} while ((index = StringPrototypeIndexOf(path, ':', index + 1)) !== -1);
409+
}
392410
if (device === undefined) {
393411
return isAbsolute ? `\\${tail}` : tail;
394412
}

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
@@ -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)