Skip to content

Commit 036e11f

Browse files
committed
Ensure relative paths are rooted
1 parent 6b3bc96 commit 036e11f

2 files changed

Lines changed: 82 additions & 12 deletions

File tree

src/AppInstallerCLITests/Filesystem.cpp

Lines changed: 63 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,69 @@ using namespace TestCommon;
1212

1313
TEST_CASE("PathEscapesDirectory", "[filesystem]")
1414
{
15-
std::string badRelativePath = "../../target.exe";
16-
std::string badRelativePath2 = "test/../../target.exe";
17-
std::string goodRelativePath = "target.exe";
18-
std::string goodRelativePath2 = "test/../test1/target.exe";
19-
20-
REQUIRE(PathEscapesBaseDirectory(badRelativePath));
21-
REQUIRE(PathEscapesBaseDirectory(badRelativePath2));
22-
REQUIRE_FALSE(PathEscapesBaseDirectory(goodRelativePath));
23-
REQUIRE_FALSE(PathEscapesBaseDirectory(goodRelativePath2));
15+
SECTION("Simple relative paths stay within the base directory")
16+
{
17+
REQUIRE_FALSE(PathEscapesBaseDirectory("target.exe"));
18+
REQUIRE_FALSE(PathEscapesBaseDirectory("test\\target.exe"));
19+
REQUIRE_FALSE(PathEscapesBaseDirectory("test/subdir/target.exe"));
20+
}
21+
22+
SECTION("Relative paths whose '..' components resolve back inside do not escape")
23+
{
24+
REQUIRE_FALSE(PathEscapesBaseDirectory("test/../test1/target.exe"));
25+
REQUIRE_FALSE(PathEscapesBaseDirectory("./target.exe"));
26+
REQUIRE_FALSE(PathEscapesBaseDirectory("a/b/../../c.exe"));
27+
}
28+
29+
SECTION("Paths that resolve to the base directory itself do not escape")
30+
{
31+
REQUIRE_FALSE(PathEscapesBaseDirectory("."));
32+
REQUIRE_FALSE(PathEscapesBaseDirectory("test/.."));
33+
}
34+
35+
SECTION("An empty path refers to the base directory itself and does not escape")
36+
{
37+
REQUIRE_FALSE(PathEscapesBaseDirectory(""));
38+
}
39+
40+
SECTION("Relative paths that traverse above the base directory escape")
41+
{
42+
REQUIRE(PathEscapesBaseDirectory("../../target.exe"));
43+
REQUIRE(PathEscapesBaseDirectory("test/../../target.exe"));
44+
REQUIRE(PathEscapesBaseDirectory("../target.exe"));
45+
REQUIRE(PathEscapesBaseDirectory(".."));
46+
REQUIRE(PathEscapesBaseDirectory("a/../../b.exe"));
47+
48+
// Mixed separators are still normalized correctly.
49+
REQUIRE(PathEscapesBaseDirectory("test\\../..\\target.exe"));
50+
}
51+
52+
SECTION("Absolute paths escape the base directory")
53+
{
54+
REQUIRE(PathEscapesBaseDirectory("C:\\Windows\\target.exe"));
55+
REQUIRE(PathEscapesBaseDirectory("C:/Windows/target.exe"));
56+
}
57+
58+
SECTION("UNC paths in their various forms escape the base directory")
59+
{
60+
REQUIRE(PathEscapesBaseDirectory("\\\\server\\share\\target.exe"));
61+
REQUIRE(PathEscapesBaseDirectory("//server/share/target.exe"));
62+
63+
// Extended-length prefix.
64+
REQUIRE(PathEscapesBaseDirectory("\\\\?\\C:\\target.exe"));
65+
}
66+
67+
SECTION("Root-relative paths (no drive) resolve to the root of the base directory's drive")
68+
{
69+
REQUIRE(PathEscapesBaseDirectory("\\Windows\\target.exe"));
70+
REQUIRE(PathEscapesBaseDirectory("/Windows/target.exe"));
71+
}
72+
73+
SECTION("Drive-relative paths resolve against the current directory of the given drive")
74+
{
75+
REQUIRE(PathEscapesBaseDirectory("C:target.exe"));
76+
REQUIRE(PathEscapesBaseDirectory("C:"));
77+
}
2478
}
2579

2680
TEST_CASE("VerifySymlink", "[filesystem]")

src/AppInstallerSharedLib/Filesystem.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,9 +372,25 @@ namespace AppInstaller::Filesystem
372372

373373
bool PathEscapesBaseDirectory(std::string_view relativePath)
374374
{
375-
// Normalize the path, then check if the first part is ".."
376-
auto resolvedPath = std::filesystem::path{ relativePath }.lexically_normal();
377-
return !resolvedPath.empty() && *resolvedPath.begin() == "..";
375+
std::filesystem::path path{ relativePath };
376+
377+
// Reject any path that has a root component. This covers absolute paths (e.g. "C:\foo"),
378+
// drive-relative paths (e.g. "C:foo") and root-relative paths (e.g. "\foo").
379+
if (path.has_root_path())
380+
{
381+
return true;
382+
}
383+
384+
// Resolve any "." and ".." components lexically (i.e. without touching the filesystem).
385+
auto resolvedPath = path.lexically_normal();
386+
387+
// If the normalized path still begins with "..", it points to a parent of the base directory.
388+
if (!resolvedPath.empty() && *resolvedPath.begin() == "..")
389+
{
390+
return true;
391+
}
392+
393+
return false;
378394
}
379395

380396
// Complicated rename algorithm due to somewhat arbitrary failures.

0 commit comments

Comments
 (0)