Skip to content

Commit 9cc89f5

Browse files
huseyinacacak-janearichardlau
authored andcommitted
path,win: fix bug in resolve and normalize
Fixes: #54025 PR-URL: #55623 Backport-PR-URL: #59831 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
1 parent 98e399b commit 9cc89f5

File tree

6 files changed

+49
-23
lines changed

6 files changed

+49
-23
lines changed

lib/path.js

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -291,10 +291,16 @@ const win32 = {
291291
j++;
292292
}
293293
if (j === len || j !== last) {
294-
// We matched a UNC root
295-
device =
296-
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
297-
rootEnd = j;
294+
if (firstPart !== '.' && firstPart !== '?') {
295+
// We matched a UNC root
296+
device =
297+
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
298+
rootEnd = j;
299+
} else {
300+
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
301+
device = `\\\\${firstPart}`;
302+
rootEnd = 4;
303+
}
298304
}
299305
}
300306
}
@@ -404,17 +410,22 @@ const win32 = {
404410
!isPathSeparator(StringPrototypeCharCodeAt(path, j))) {
405411
j++;
406412
}
407-
if (j === len) {
408-
// We matched a UNC root only
409-
// Return the normalized version of the UNC root since there
410-
// is nothing left to process
411-
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
412-
}
413-
if (j !== last) {
414-
// We matched a UNC root with leftovers
415-
device =
416-
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
417-
rootEnd = j;
413+
if (j === len || j !== last) {
414+
if (firstPart === '.' || firstPart === '?') {
415+
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
416+
device = `\\\\${firstPart}`;
417+
rootEnd = 4;
418+
} else if (j === len) {
419+
// We matched a UNC root only
420+
// Return the normalized version of the UNC root since there
421+
// is nothing left to process
422+
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
423+
} else {
424+
// We matched a UNC root with leftovers
425+
device =
426+
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
427+
rootEnd = j;
428+
}
418429
}
419430
}
420431
}

src/path.cc

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,16 @@ std::string PathResolve(Environment* env,
170170
j++;
171171
}
172172
if (j == len || j != last) {
173-
// We matched a UNC root
174-
device = "\\\\" + firstPart + "\\" + path.substr(last, j - last);
175-
rootEnd = j;
173+
if (firstPart != "." && firstPart != "?") {
174+
// We matched a UNC root
175+
device =
176+
"\\\\" + firstPart + "\\" + path.substr(last, j - last);
177+
rootEnd = j;
178+
} else {
179+
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
180+
device = "\\\\" + firstPart;
181+
rootEnd = 4;
182+
}
176183
}
177184
}
178185
}

test/cctest/test_path.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ TEST_F(PathTest, PathResolve) {
3939
EXPECT_EQ(
4040
PathResolve(*env, {"C:\\foo\\tmp.3\\", "..\\tmp.3\\cycles\\root.js"}),
4141
"C:\\foo\\tmp.3\\cycles\\root.js");
42+
EXPECT_EQ(PathResolve(*env, {"\\\\.\\PHYSICALDRIVE0"}),
43+
"\\\\.\\PHYSICALDRIVE0");
44+
EXPECT_EQ(PathResolve(*env, {"\\\\?\\PHYSICALDRIVE0"}),
45+
"\\\\?\\PHYSICALDRIVE0");
4246
#else
4347
EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file");
4448
EXPECT_EQ(PathResolve(*env, {"/var/lib", "/../", "file/"}), "/file");

test/parallel/test-path-makelong.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ assert.strictEqual(path.win32.toNamespacedPath('\\\\foo\\bar'),
7979
'\\\\?\\UNC\\foo\\bar\\');
8080
assert.strictEqual(path.win32.toNamespacedPath('//foo//bar'),
8181
'\\\\?\\UNC\\foo\\bar\\');
82-
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\foo'), '\\\\?\\foo\\');
82+
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\foo'), '\\\\?\\foo');
8383
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\c:\\Windows/System'), '\\\\?\\c:\\Windows\\System');
8484
assert.strictEqual(path.win32.toNamespacedPath(null), null);
8585
assert.strictEqual(path.win32.toNamespacedPath(true), true);

test/parallel/test-path-normalize.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ assert.strictEqual(
4040
'..\\..\\..\\..\\baz'
4141
);
4242
assert.strictEqual(path.win32.normalize('foo/bar\\baz'), 'foo\\bar\\baz');
43+
assert.strictEqual(path.win32.normalize('\\\\.\\foo'), '\\\\.\\foo');
44+
assert.strictEqual(path.win32.normalize('\\\\.\\foo\\'), '\\\\.\\foo\\');
4345

4446
// Tests related to CVE-2024-36139. Path traversal should not result in changing
4547
// the root directory on Windows.
@@ -58,10 +60,10 @@ assert.strictEqual(path.win32.normalize('/test/../??/D:/Test'), '\\??\\D:\\Test'
5860
assert.strictEqual(path.win32.normalize('/test/../?/D:/Test'), '\\?\\D:\\Test');
5961
assert.strictEqual(path.win32.normalize('//test/../??/D:/Test'), '\\\\test\\..\\??\\D:\\Test');
6062
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');
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');
6567
assert.strictEqual(path.win32.normalize('//server/share/dir/../../../?/D:/file'),
6668
'\\\\server\\share\\?\\D:\\file');
6769
assert.strictEqual(path.win32.normalize('//server/goodshare/../badshare/file'),

test/parallel/test-path-resolve.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ const resolveTests = [
3535
[['c:/', '///some//dir'], 'c:\\some\\dir'],
3636
[['C:\\foo\\tmp.3\\', '..\\tmp.3\\cycles\\root.js'],
3737
'C:\\foo\\tmp.3\\cycles\\root.js'],
38+
[['\\\\.\\PHYSICALDRIVE0'], '\\\\.\\PHYSICALDRIVE0'],
39+
[['\\\\?\\PHYSICALDRIVE0'], '\\\\?\\PHYSICALDRIVE0'],
3840
],
3941
],
4042
[ path.posix.resolve,

0 commit comments

Comments
 (0)