Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 63 additions & 9 deletions src/AppInstallerCLITests/Filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,69 @@ using namespace TestCommon;

TEST_CASE("PathEscapesDirectory", "[filesystem]")
{
std::string badRelativePath = "../../target.exe";
std::string badRelativePath2 = "test/../../target.exe";
std::string goodRelativePath = "target.exe";
std::string goodRelativePath2 = "test/../test1/target.exe";

REQUIRE(PathEscapesBaseDirectory(badRelativePath));
REQUIRE(PathEscapesBaseDirectory(badRelativePath2));
REQUIRE_FALSE(PathEscapesBaseDirectory(goodRelativePath));
REQUIRE_FALSE(PathEscapesBaseDirectory(goodRelativePath2));
SECTION("Simple relative paths stay within the base directory")
{
REQUIRE_FALSE(PathEscapesBaseDirectory("target.exe"));
REQUIRE_FALSE(PathEscapesBaseDirectory("test\\target.exe"));
REQUIRE_FALSE(PathEscapesBaseDirectory("test/subdir/target.exe"));
}

SECTION("Relative paths whose '..' components resolve back inside do not escape")
{
REQUIRE_FALSE(PathEscapesBaseDirectory("test/../test1/target.exe"));
REQUIRE_FALSE(PathEscapesBaseDirectory("./target.exe"));
REQUIRE_FALSE(PathEscapesBaseDirectory("a/b/../../c.exe"));
}

SECTION("Paths that resolve to the base directory itself do not escape")
{
REQUIRE_FALSE(PathEscapesBaseDirectory("."));
REQUIRE_FALSE(PathEscapesBaseDirectory("test/.."));
}

SECTION("An empty path refers to the base directory itself and does not escape")
{
REQUIRE_FALSE(PathEscapesBaseDirectory(""));
}

SECTION("Relative paths that traverse above the base directory escape")
{
REQUIRE(PathEscapesBaseDirectory("../../target.exe"));
REQUIRE(PathEscapesBaseDirectory("test/../../target.exe"));
REQUIRE(PathEscapesBaseDirectory("../target.exe"));
REQUIRE(PathEscapesBaseDirectory(".."));
REQUIRE(PathEscapesBaseDirectory("a/../../b.exe"));

// Mixed separators are still normalized correctly.
REQUIRE(PathEscapesBaseDirectory("test\\../..\\target.exe"));
}

SECTION("Absolute paths escape the base directory")
{
REQUIRE(PathEscapesBaseDirectory("C:\\Windows\\target.exe"));
REQUIRE(PathEscapesBaseDirectory("C:/Windows/target.exe"));
}

SECTION("UNC paths in their various forms escape the base directory")
{
REQUIRE(PathEscapesBaseDirectory("\\\\server\\share\\target.exe"));
REQUIRE(PathEscapesBaseDirectory("//server/share/target.exe"));

// Extended-length prefix.
REQUIRE(PathEscapesBaseDirectory("\\\\?\\C:\\target.exe"));
}

SECTION("Root-relative paths (no drive) resolve to the root of the base directory's drive")
{
REQUIRE(PathEscapesBaseDirectory("\\Windows\\target.exe"));
REQUIRE(PathEscapesBaseDirectory("/Windows/target.exe"));
}

SECTION("Drive-relative paths resolve against the current directory of the given drive")
{
REQUIRE(PathEscapesBaseDirectory("C:target.exe"));
REQUIRE(PathEscapesBaseDirectory("C:"));
}
}

TEST_CASE("VerifySymlink", "[filesystem]")
Expand Down
22 changes: 19 additions & 3 deletions src/AppInstallerSharedLib/Filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,25 @@ namespace AppInstaller::Filesystem

bool PathEscapesBaseDirectory(std::string_view relativePath)
{
// Normalize the path, then check if the first part is ".."
auto resolvedPath = std::filesystem::path{ relativePath }.lexically_normal();
return !resolvedPath.empty() && *resolvedPath.begin() == "..";
std::filesystem::path path{ relativePath };

// Reject any path that has a root component. This covers absolute paths (e.g. "C:\foo"),
// drive-relative paths (e.g. "C:foo") and root-relative paths (e.g. "\foo").
if (path.has_root_path())
{
return true;
}

// Resolve any "." and ".." components lexically (i.e. without touching the filesystem).
auto resolvedPath = path.lexically_normal();

// If the normalized path still begins with "..", it points to a parent of the base directory.
if (!resolvedPath.empty() && *resolvedPath.begin() == "..")
{
return true;
}

return false;
}

// Complicated rename algorithm due to somewhat arbitrary failures.
Expand Down
Loading