Ensure portable command alias does not escape directory

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;

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj

@@ -562,6 +562,9 @@
     <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-DuplicateRelativeFilePath.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-InvalidPortableCommandAlias.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters

@@ -507,6 +507,9 @@
     <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-DuplicateRelativeFilePath.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-InvalidPortableCommandAlias.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>

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]")

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

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

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." },

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);
+                                }
                             }
                         }
                     }

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);

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.

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);

src/WinGetUtilInterop/Common/ManifestErrorId.cs

@@ -159,6 +159,9 @@ public enum ManifestErrorId
         /// <summary>Optional field missing.</summary>
         OptionalFieldMissing,
 
+        /// <summary>Portable command alias must not point to a location outside of base directory.</summary>
+        PortableCommandAliasEscapesDirectory,
+
         /// <summary>Relative file path must not point to a location outside of archive directory.</summary>
         RelativeFilePathEscapesDirectory,