diff --git a/src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp b/src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp
index 680c0f1e15..9f2325323b 100644
--- a/src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp
+++ b/src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp
@@ -92,7 +92,7 @@ namespace AppInstaller::CLI::Workflow
{
const std::filesystem::path& nestedInstallerPath = targetInstallerPath / ConvertToUTF16(nestedInstallerFile.RelativeFilePath);
- if (Filesystem::PathEscapesBaseDirectory(nestedInstallerPath, targetInstallerPath))
+ if (Filesystem::PathEscapesBaseDirectory(nestedInstallerFile.RelativeFilePath))
{
AICLI_LOG(CLI, Error, << "Path points to a location outside of the install directory: " << nestedInstallerPath);
context.Reporter.Error() << Resource::String::InvalidPathToNestedInstaller << std::endl;
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
index b8bc46728c..8dea7d9f4a 100644
--- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
+++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
@@ -562,6 +562,9 @@
true
+
+ true
+
true
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
index 61ae5cfea0..348f9cdfda 100644
--- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
+++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
@@ -507,6 +507,9 @@
TestData
+
+ TestData
+
TestData
diff --git a/src/AppInstallerCLITests/Filesystem.cpp b/src/AppInstallerCLITests/Filesystem.cpp
index 733f38f08b..59cf02b840 100644
--- a/src/AppInstallerCLITests/Filesystem.cpp
+++ b/src/AppInstallerCLITests/Filesystem.cpp
@@ -12,23 +12,15 @@ using namespace TestCommon;
TEST_CASE("PathEscapesDirectory", "[filesystem]")
{
- TestCommon::TempDirectory tempDirectory("TempDirectory");
- const std::filesystem::path& basePath = tempDirectory.GetPath();
-
std::string badRelativePath = "../../target.exe";
std::string badRelativePath2 = "test/../../target.exe";
std::string goodRelativePath = "target.exe";
std::string goodRelativePath2 = "test/../test1/target.exe";
- std::filesystem::path badPath = basePath / badRelativePath;
- std::filesystem::path badPath2 = basePath / badRelativePath2;
- std::filesystem::path goodPath = basePath / goodRelativePath;
- std::filesystem::path goodPath2 = basePath / goodRelativePath2;
-
- REQUIRE(PathEscapesBaseDirectory(badPath, basePath));
- REQUIRE(PathEscapesBaseDirectory(badPath2, basePath));
- REQUIRE_FALSE(PathEscapesBaseDirectory(goodPath, basePath));
- REQUIRE_FALSE(PathEscapesBaseDirectory(goodPath2, basePath));
+ REQUIRE(PathEscapesBaseDirectory(badRelativePath));
+ REQUIRE(PathEscapesBaseDirectory(badRelativePath2));
+ REQUIRE_FALSE(PathEscapesBaseDirectory(goodRelativePath));
+ REQUIRE_FALSE(PathEscapesBaseDirectory(goodRelativePath2));
}
TEST_CASE("VerifySymlink", "[filesystem]")
diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidPortableCommandAlias.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidPortableCommandAlias.yaml
new file mode 100644
index 0000000000..eeb5b9fd72
--- /dev/null
+++ b/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidPortableCommandAlias.yaml
@@ -0,0 +1,23 @@
+# Bad manifest. A nested installer file must not have a command alias outside the base directory.
+# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.4.0.schema.json
+
+PackageIdentifier: microsoft.msixsdk
+PackageVersion: 1.0.0.0
+PackageLocale: en-US
+PackageName: AppInstaller Test Installer
+Publisher: Microsoft Corporation
+Moniker: AICLITestExe
+License: Test
+ShortDescription: Test installer for zip with a command alias outside the base directory
+Scope: user
+Installers:
+ - Architecture: x64
+ InstallerUrl: https://ThisIsNotUsed
+ InstallerType: zip
+ InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+ NestedInstallerType: exe
+ NestedInstallerFiles:
+ - RelativeFilePath: relativeFilePath
+ PortableCommandAlias: ../command-alias
+ManifestType: singleton
+ManifestVersion: 1.4.0
diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml
index d50d6c6fb9..3726d0aad0 100644
--- a/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml
+++ b/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml
@@ -1,4 +1,4 @@
-# Bad manifest. A nested installer file must have a RelativeFilePath specified.
+# Bad manifest. A nested installer file must have a valid RelativeFilePath.
# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.4.0.schema.json
PackageIdentifier: microsoft.msixsdk
@@ -8,7 +8,7 @@ PackageName: AppInstaller Test Installer
Publisher: Microsoft Corporation
Moniker: AICLITestExe
License: Test
-ShortDescription: Test installer for zip without RelativeFilePath specified
+ShortDescription: Test installer for zip with an invalid RelativeFilePath
Scope: user
Installers:
- Architecture: x64
diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp
index 420dcd9d0d..df758fe857 100644
--- a/src/AppInstallerCLITests/YamlManifest.cpp
+++ b/src/AppInstallerCLITests/YamlManifest.cpp
@@ -882,6 +882,7 @@ TEST_CASE("ReadBadManifests", "[ManifestValidation]")
{ "Manifest-Bad-InstallerTypePortable-InvalidScope.yaml", "Scope is not supported for InstallerType portable." },
{ "Manifest-Bad-InstallerTypeZip-DuplicateCommandAlias.yaml", "Duplicate portable command alias found." },
{ "Manifest-Bad-InstallerTypeZip-DuplicateRelativeFilePath.yaml", "Duplicate relative file path found." },
+ { "Manifest-Bad-InstallerTypeZip-InvalidPortableCommandAlias.yaml", "Portable command alias must not point to a location outside of base directory." },
{ "Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml", "Relative file path must not point to a location outside of archive directory" },
{ "Manifest-Bad-InstallerTypeZip-MissingRelativeFilePath.yaml", "Required field missing. [RelativeFilePath]" },
{ "Manifest-Bad-InstallerTypeZip-MultipleNestedInstallers.yaml", "Only one entry for NestedInstallerFiles can be specified for non-portable InstallerTypes." },
diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp
index e4ddd190f8..b5c04a796b 100644
--- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp
+++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp
@@ -77,6 +77,7 @@ namespace AppInstaller::Manifest
{ AppInstaller::Manifest::ManifestError::ArpVersionOverlapWithIndex, "DisplayVersion declared in the manifest has overlap with existing DisplayVersion range in the index. Existing DisplayVersion range in index: "sv },
{ AppInstaller::Manifest::ManifestError::ArpVersionValidationInternalError, "Internal error while validating DisplayVersion against index."sv },
{ AppInstaller::Manifest::ManifestError::ExceededNestedInstallerFilesLimit, "Only one entry for NestedInstallerFiles can be specified for non-portable InstallerTypes."sv },
+ { AppInstaller::Manifest::ManifestError::PortableCommandAliasEscapesDirectory, "Portable command alias must not point to a location outside of base directory."sv },
{ AppInstaller::Manifest::ManifestError::RelativeFilePathEscapesDirectory, "Relative file path must not point to a location outside of archive directory."sv },
{ AppInstaller::Manifest::ManifestError::ArpValidationError, "Arp Validation Error."sv },
{ AppInstaller::Manifest::ManifestError::SchemaError, "Schema Error."sv },
@@ -349,9 +350,7 @@ namespace AppInstaller::Manifest
}
// Check that the relative file path does not escape base directory.
- const std::filesystem::path& basePath = std::filesystem::current_path();
- const std::filesystem::path& fullPath = basePath / ConvertToUTF16(nestedInstallerFile.RelativeFilePath);
- if (AppInstaller::Filesystem::PathEscapesBaseDirectory(fullPath, basePath))
+ if (AppInstaller::Filesystem::PathEscapesBaseDirectory(nestedInstallerFile.RelativeFilePath))
{
resultErrors.emplace_back(ManifestError::RelativeFilePathEscapesDirectory, "RelativeFilePath");
}
@@ -362,32 +361,45 @@ namespace AppInstaller::Manifest
resultErrors.emplace_back(ManifestError::DuplicateRelativeFilePath, "RelativeFilePath");
}
- // Check for duplicate portable command alias values.
- const auto& alias = Utility::ToLower(nestedInstallerFile.PortableCommandAlias);
- if (!alias.empty() && !commandAliasSet.insert(alias).second)
+ if (!nestedInstallerFile.PortableCommandAlias.empty())
{
- resultErrors.emplace_back(ManifestError::DuplicatePortableCommandAlias, "PortableCommandAlias");
- break;
+ // Check that the command alias does not escape the base directory.
+ if (AppInstaller::Filesystem::PathEscapesBaseDirectory(nestedInstallerFile.PortableCommandAlias))
+ {
+ resultErrors.emplace_back(ManifestError::PortableCommandAliasEscapesDirectory, "PortableCommandAlias");
+ }
+
+ // Check for duplicate portable command alias values.
+ const auto& alias = Utility::ToLower(nestedInstallerFile.PortableCommandAlias);
+ if (!commandAliasSet.insert(alias).second)
+ {
+ resultErrors.emplace_back(ManifestError::DuplicatePortableCommandAlias, "PortableCommandAlias");
+ break;
+ }
}
// If running full validation, check filetype
if (options.FullValidation)
{
- const std::wstring lowerExtension = Utility::ToLower(fullPath.extension().wstring());
-
- if (isPortable)
+ std::filesystem::path relativeFilePath{ Utility::ConvertToUTF16(nestedInstallerFile.RelativeFilePath) };
+ if (relativeFilePath.has_extension())
{
- if (fullPath.has_extension() && std::find(s_AllowedPortableFiletypes.begin(), s_AllowedPortableFiletypes.end(), lowerExtension) == s_AllowedPortableFiletypes.end())
+ const std::wstring lowerExtension = Utility::ToLower(relativeFilePath.extension().wstring());
+
+ if (isPortable)
{
- resultErrors.emplace_back(ManifestError::InvalidPortableFiletype, "RelativeFilePath", nestedInstallerFile.RelativeFilePath);
+ if (std::find(s_AllowedPortableFiletypes.begin(), s_AllowedPortableFiletypes.end(), lowerExtension) == s_AllowedPortableFiletypes.end())
+ {
+ resultErrors.emplace_back(ManifestError::InvalidPortableFiletype, "RelativeFilePath", nestedInstallerFile.RelativeFilePath);
+ }
}
- }
- if (isFont)
- {
- if (fullPath.has_extension() && std::find(s_AllowedFontFiletypes.begin(), s_AllowedFontFiletypes.end(), lowerExtension) == s_AllowedFontFiletypes.end())
+ if (isFont)
{
- resultErrors.emplace_back(ManifestError::InvalidFontFiletype, "RelativeFilePath", nestedInstallerFile.RelativeFilePath);
+ if (std::find(s_AllowedFontFiletypes.begin(), s_AllowedFontFiletypes.end(), lowerExtension) == s_AllowedFontFiletypes.end())
+ {
+ resultErrors.emplace_back(ManifestError::InvalidFontFiletype, "RelativeFilePath", nestedInstallerFile.RelativeFilePath);
+ }
}
}
}
diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h
index c701df3b9c..55eef941ac 100644
--- a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h
+++ b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h
@@ -68,6 +68,7 @@ namespace AppInstaller::Manifest
WINGET_DEFINE_RESOURCE_STRINGID(NoSuitableMinVersionDependency);
WINGET_DEFINE_RESOURCE_STRINGID(NoSupportedPlatforms);
WINGET_DEFINE_RESOURCE_STRINGID(OptionalFieldMissing);
+ WINGET_DEFINE_RESOURCE_STRINGID(PortableCommandAliasEscapesDirectory);
WINGET_DEFINE_RESOURCE_STRINGID(RelativeFilePathEscapesDirectory);
WINGET_DEFINE_RESOURCE_STRINGID(RequiredFieldEmpty);
WINGET_DEFINE_RESOURCE_STRINGID(RequiredFieldMissing);
diff --git a/src/AppInstallerSharedLib/Filesystem.cpp b/src/AppInstallerSharedLib/Filesystem.cpp
index 3c67411c80..2e0e283359 100644
--- a/src/AppInstallerSharedLib/Filesystem.cpp
+++ b/src/AppInstallerSharedLib/Filesystem.cpp
@@ -370,12 +370,11 @@ namespace AppInstaller::Filesystem
return (GetVolumeInformationFlags(path) & FILE_SUPPORTS_REPARSE_POINTS) != 0;
}
- bool PathEscapesBaseDirectory(const std::filesystem::path& target, const std::filesystem::path& base)
+ bool PathEscapesBaseDirectory(std::string_view relativePath)
{
- const auto& targetPath = std::filesystem::weakly_canonical(target);
- const auto& basePath = std::filesystem::weakly_canonical(base);
- auto [a, b] = std::mismatch(targetPath.begin(), targetPath.end(), basePath.begin(), basePath.end());
- return (b != basePath.end());
+ // Normalize the path, then check if the first part is ".."
+ auto resolvedPath = std::filesystem::path{ relativePath }.lexically_normal();
+ return !resolvedPath.empty() && *resolvedPath.begin() == "..";
}
// Complicated rename algorithm due to somewhat arbitrary failures.
diff --git a/src/AppInstallerSharedLib/Public/winget/Filesystem.h b/src/AppInstallerSharedLib/Public/winget/Filesystem.h
index c833e8e921..1a7df6aa28 100644
--- a/src/AppInstallerSharedLib/Public/winget/Filesystem.h
+++ b/src/AppInstallerSharedLib/Public/winget/Filesystem.h
@@ -21,8 +21,8 @@ namespace AppInstaller::Filesystem
// Checks if the file system at path support reparse points
bool SupportsReparsePoints(const std::filesystem::path& path);
- // Checks if the canonical form of the path points to a location outside of the provided base path.
- bool PathEscapesBaseDirectory(const std::filesystem::path& target, const std::filesystem::path& base);
+ // Checks if a relative paths points to a location outside of the base path.
+ bool PathEscapesBaseDirectory(std::string_view relativePath);
// Renames the file to a new path.
void RenameFile(const std::filesystem::path& from, const std::filesystem::path& to);
diff --git a/src/WinGetUtilInterop/Common/ManifestErrorId.cs b/src/WinGetUtilInterop/Common/ManifestErrorId.cs
index 43456c613c..607bce3d2e 100644
--- a/src/WinGetUtilInterop/Common/ManifestErrorId.cs
+++ b/src/WinGetUtilInterop/Common/ManifestErrorId.cs
@@ -159,6 +159,9 @@ public enum ManifestErrorId
/// Optional field missing.
OptionalFieldMissing,
+ /// Portable command alias must not point to a location outside of base directory.
+ PortableCommandAliasEscapesDirectory,
+
/// Relative file path must not point to a location outside of archive directory.
RelativeFilePathEscapesDirectory,