diff --git a/doc/ReleaseNotes.md b/doc/ReleaseNotes.md
index 4b81aabb85..e5be826943 100644
--- a/doc/ReleaseNotes.md
+++ b/doc/ReleaseNotes.md
@@ -6,3 +6,4 @@ Nothing yet.
* Updated NUnit to v4
* Fixed a crash (`0x8000ffff`) when using `--disable-interactivity` with the Resume experimental feature enabled during install operations.
+* Fixed relative path handling for rooted paths.
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
index 093eb5b235..7bd2310985 100644
--- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
+++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
@@ -570,6 +570,9 @@
true
+
+ true
+
true
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
index 29debf6a13..5fc2e79a90 100644
--- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
+++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
@@ -522,6 +522,9 @@
TestData
+
+ TestData
+
TestData
diff --git a/src/AppInstallerCLITests/Filesystem.cpp b/src/AppInstallerCLITests/Filesystem.cpp
index 12684f19d5..95ea62c493 100644
--- a/src/AppInstallerCLITests/Filesystem.cpp
+++ b/src/AppInstallerCLITests/Filesystem.cpp
@@ -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]")
diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypePortable-InvalidCommandAlias.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypePortable-InvalidCommandAlias.yaml
new file mode 100644
index 0000000000..d8d3104a62
--- /dev/null
+++ b/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypePortable-InvalidCommandAlias.yaml
@@ -0,0 +1,20 @@
+# Bad manifest. A portable installer must not have a command alias outside the base directory.
+# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.2.0.schema.json
+
+PackageIdentifier: TestInstaller.WithLicenseAgreement
+PackageVersion: 1.0.0.0
+PackageLocale: en-US
+PackageName: AppInstaller Test Installer
+Publisher: Microsoft Corporation
+Moniker: AICLITestExe
+License: Test
+ShortDescription: Test installer for portable with a command alias outside the base directory
+Commands:
+ - ../command-alias
+Installers:
+ - Architecture: x64
+ InstallerUrl: https://ThisIsNotUsed
+ InstallerType: portable
+ InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+ManifestType: singleton
+ManifestVersion: 1.2.0
diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp
index 60f997e782..796c4d15fa 100644
--- a/src/AppInstallerCLITests/YamlManifest.cpp
+++ b/src/AppInstallerCLITests/YamlManifest.cpp
@@ -878,6 +878,7 @@ TEST_CASE("ReadBadManifests", "[ManifestValidation]")
{ "Manifest-Bad-InstallerTypeInvalid.yaml", "Invalid field value. [InstallerType]" },
{ "Manifest-Bad-InstallerTypeMissing.yaml", "Invalid field value. [InstallerType]" },
{ "Manifest-Bad-InstallerTypePortable-InvalidAppsAndFeatures.yaml", "Only zero or one entry for Apps and Features may be specified for InstallerType portable." },
+ { "Manifest-Bad-InstallerTypePortable-InvalidCommandAlias.yaml", "Portable command alias must not point to a location outside of base directory." },
{ "Manifest-Bad-InstallerTypePortable-InvalidCommands.yaml", "Only zero or one value for Commands may be specified for InstallerType portable." },
{ "Manifest-Bad-InstallerTypePortable-InvalidScope.yaml", "Scope is not supported for InstallerType portable." },
{ "Manifest-Bad-InstallerTypeZip-DuplicateCommandAlias.yaml", "Duplicate portable command alias found." },
diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp
index 57573d151d..bfe866f0ed 100644
--- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp
+++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp
@@ -306,6 +306,14 @@ namespace AppInstaller::Manifest
{
resultErrors.emplace_back(ManifestError::ExceededCommandsLimit);
}
+
+ if (!installer.Commands.empty())
+ {
+ if (AppInstaller::Filesystem::PathEscapesBaseDirectory(installer.Commands[0]))
+ {
+ resultErrors.emplace_back(ManifestError::PortableCommandAliasEscapesDirectory, "Commands");
+ }
+ }
}
if (installer.EffectiveInstallerType() == InstallerTypeEnum::Portable)
diff --git a/src/AppInstallerSharedLib/Filesystem.cpp b/src/AppInstallerSharedLib/Filesystem.cpp
index 85acbf09a3..90394c6824 100644
--- a/src/AppInstallerSharedLib/Filesystem.cpp
+++ b/src/AppInstallerSharedLib/Filesystem.cpp
@@ -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.