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: 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
This commit is contained in:
RafaelGSS 2025-01-20 11:40:16 -03:00
parent 1b693fa03a
commit 8306457110
3 changed files with 51 additions and 0 deletions

View File

@ -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;
}

View File

@ -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'],
]
),
]);

View File

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