Skip to content

Commit 183353a

Browse files
huseyinacacak-janeamarco-ippolito
authored andcommitted
path,win: fix bug in resolve and normalize
Fixes: #54025 PR-URL: #55623 Backport-PR-URL: #59831 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 6f580d5 commit 183353a

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
@@ -285,10 +285,16 @@ const win32 = {
285285
j++;
286286
}
287287
if (j === len || j !== last) {
288-
// We matched a UNC root
289-
device =
290-
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
291-
rootEnd = j;
288+
if (firstPart !== '.' && firstPart !== '?') {
289+
// We matched a UNC root
290+
device =
291+
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
292+
rootEnd = j;
293+
} else {
294+
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
295+
device = `\\\\${firstPart}`;
296+
rootEnd = 4;
297+
}
292298
}
293299
}
294300
}
@@ -398,17 +404,22 @@ const win32 = {
398404
!isPathSeparator(StringPrototypeCharCodeAt(path, j))) {
399405
j++;
400406
}
401-
if (j === len) {
402-
// We matched a UNC root only
403-
// Return the normalized version of the UNC root since there
404-
// is nothing left to process
405-
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
406-
}
407-
if (j !== last) {
408-
// We matched a UNC root with leftovers
409-
device =
410-
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
411-
rootEnd = j;
407+
if (j === len || j !== last) {
408+
if (firstPart === '.' || firstPart === '?') {
409+
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
410+
device = `\\\\${firstPart}`;
411+
rootEnd = 4;
412+
} else if (j === len) {
413+
// We matched a UNC root only
414+
// Return the normalized version of the UNC root since there
415+
// is nothing left to process
416+
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
417+
} else {
418+
// We matched a UNC root with leftovers
419+
device =
420+
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
421+
rootEnd = j;
422+
}
412423
}
413424
}
414425
}

src/path.cc

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

test/cctest/test_path.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ TEST_F(PathTest, PathResolve) {
3535
EXPECT_EQ(
3636
PathResolve(*env, {"C:\\foo\\tmp.3\\", "..\\tmp.3\\cycles\\root.js"}),
3737
"C:\\foo\\tmp.3\\cycles\\root.js");
38+
EXPECT_EQ(PathResolve(*env, {"\\\\.\\PHYSICALDRIVE0"}),
39+
"\\\\.\\PHYSICALDRIVE0");
40+
EXPECT_EQ(PathResolve(*env, {"\\\\?\\PHYSICALDRIVE0"}),
41+
"\\\\?\\PHYSICALDRIVE0");
3842
#else
3943
EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file");
4044
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
@@ -33,6 +33,8 @@ const resolveTests = [
3333
[['c:/', '///some//dir'], 'c:\\some\\dir'],
3434
[['C:\\foo\\tmp.3\\', '..\\tmp.3\\cycles\\root.js'],
3535
'C:\\foo\\tmp.3\\cycles\\root.js'],
36+
[['\\\\.\\PHYSICALDRIVE0'], '\\\\.\\PHYSICALDRIVE0'],
37+
[['\\\\?\\PHYSICALDRIVE0'], '\\\\?\\PHYSICALDRIVE0'],
3638
],
3739
],
3840
[ path.posix.resolve,

0 commit comments

Comments
 (0)