Prevent access to parent directories from relativeFilePath for archive install

src/AppInstallerCLICore/Resources.h

@@ -159,6 +159,7 @@ namespace AppInstaller::CLI::Resource
         WINGET_DEFINE_RESOURCE_STRINGID(InvalidArgumentWithoutQueryError);
         WINGET_DEFINE_RESOURCE_STRINGID(InvalidJsonFile);
         WINGET_DEFINE_RESOURCE_STRINGID(InvalidNameError);
+        WINGET_DEFINE_RESOURCE_STRINGID(InvalidRelativeFilePathToNestedInstaller);
         WINGET_DEFINE_RESOURCE_STRINGID(LicenseAgreement);
         WINGET_DEFINE_RESOURCE_STRINGID(Links);
         WINGET_DEFINE_RESOURCE_STRINGID(ListCommandLongDescription);
@@ -190,6 +191,7 @@ namespace AppInstaller::CLI::Resource
         WINGET_DEFINE_RESOURCE_STRINGID(MultiplePackagesFound);
         WINGET_DEFINE_RESOURCE_STRINGID(NameArgumentDescription);
         WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotFound);
+        WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotSpecified);
         WINGET_DEFINE_RESOURCE_STRINGID(NoApplicableInstallers);
         WINGET_DEFINE_RESOURCE_STRINGID(NoExperimentalFeaturesMessage);
         WINGET_DEFINE_RESOURCE_STRINGID(NoInstalledPackageFound);

src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp

@@ -4,8 +4,18 @@
 #include "ArchiveFlow.h"
 #include "winget/Archive.h"
 
+using namespace AppInstaller::Manifest;
+
 namespace AppInstaller::CLI::Workflow
 {
+    namespace
+    {
+        bool RelativePathContainsEscapeString(const std::string& relativePath)
+        {
+            return relativePath.find("..") != std::string::npos;
+        }
+    }
+
     void ExtractFilesFromArchive(Execution::Context& context)
     {
         const auto& installerPath = context.Get<Execution::Data::InstallerPath>();
@@ -32,7 +42,7 @@ namespace AppInstaller::CLI::Workflow
         const auto& installer = context.Get<Execution::Data::Installer>().value();
         if (installer.NestedInstallerFiles.empty())
         {
-            // Manifest validation should prevent this from happening
+            // Pre-install validation should prevent this from happening
             AICLI_LOG(CLI, Error, << "No entries specified for NestedInstallerFiles");
             AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INVALID_MANIFEST);
         }
@@ -56,4 +66,31 @@ namespace AppInstaller::CLI::Workflow
             context.Add<Execution::Data::InstallerPath>(nestedInstallerPath);
         }
     }
+
+    void EnsureValidNestedInstallerMetadataForArchiveInstall(Execution::Context& context)
+    {
+        auto installer = context.Get<Execution::Data::Installer>().value();
+
+        if (installer.InstallerType == InstallerTypeEnum::Zip)
+        {
+            auto const& nestedInstallerFiles = installer.NestedInstallerFiles;
+            if (nestedInstallerFiles.empty())
+            {
+                AICLI_LOG(CLI, Error, << "No entries specified for NestedInstallerFiles");
+                context.Reporter.Error() << Resource::String::NestedInstallerNotSpecified << std::endl;
+                AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INVALID_MANIFEST);
+            }
+
+            for (auto nestedInstallerFile : nestedInstallerFiles)
+            {
+                auto const& relativeFilePath = nestedInstallerFile.RelativeFilePath;
+                if (RelativePathContainsEscapeString(relativeFilePath))
+                {
+                    AICLI_LOG(CLI, Error, << "RelativeFilePath contains escape string: " << relativeFilePath);
+                    context.Reporter.Error() << Resource::String::InvalidRelativeFilePathToNestedInstaller << std::endl;
+                    AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NESTEDINSTALLER_INVALID_PATH);
+                }
+            }
+        }
+    }
 }

src/AppInstallerCLICore/Workflows/ArchiveFlow.h

@@ -16,4 +16,10 @@ namespace AppInstaller::CLI::Workflow
     // Inputs: Installer, InstallerPath
     // Outputs: None
     void VerifyAndSetNestedInstaller(Execution::Context& context);
+
+    // Verifies that the metadata related to the NestedInstaller is valid
+    // Required Args: None
+    // Inputs: Installer, InstallerPath
+    // Outputs: None
+    void EnsureValidNestedInstallerMetadataForArchiveInstall(Execution::Context& context);
 }

src/AppInstallerCLICore/Workflows/InstallFlow.cpp

@@ -542,7 +542,8 @@ namespace AppInstaller::CLI::Workflow
     {
         context <<
             Workflow::EnsureSupportForPortableInstall <<
-            Workflow::EnsureNonPortableTypeForArchiveInstall;
+            Workflow::EnsureNonPortableTypeForArchiveInstall <<
+            Workflow::EnsureValidNestedInstallerMetadataForArchiveInstall;
     }
 
     void InstallMultiplePackages::operator()(Execution::Context& context) const

src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

@@ -1370,4 +1370,10 @@ Please specify one of them using the `--source` option to proceed.</value>
   <data name="NestedInstallerNotFound" xml:space="preserve">
     <value>Nested installer file does not exist. Ensure the specified relative path of the nested installer matches: </value>
   </data>
+  <data name="InvalidRelativeFilePathToNestedInstaller" xml:space="preserve">
+    <value>Invalid relative file path to nested installer</value>
+  </data>
+  <data name="NestedInstallerNotSpecified" xml:space="preserve">
+    <value>No nested installers specified for this package</value>
+  </data>
 </root>

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj

@@ -286,6 +286,12 @@
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\InstallFlowTest_ZipWithExe.yaml">
       <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_InvalidRelativeFilePath.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_MissingNestedInstaller.yaml">
+      <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>    
     <CopyFileToFolders Include="TestData\ImportFile-Bad-Invalid.json">
       <DeploymentContent>true</DeploymentContent>

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters

@@ -509,6 +509,12 @@
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\InstallFlowTest_ZipWithExe.yaml">
       <Filter>TestData</Filter>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_InvalidRelativeFilePath.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_MissingNestedInstaller.yaml">
+      <Filter>TestData</Filter>
     </CopyFileToFolders>    
     <CopyFileToFolders Include="TestData\InstallerArgTest_Msi_WithSwitches.yaml">
       <Filter>TestData</Filter>

src/AppInstallerCLITests/TestData/InstallFlowTest_Zip_InvalidRelativeFilePath.yaml

@@ -0,0 +1,23 @@
+PackageIdentifier: AppInstallerCliTest.TestZipInstaller
+PackageVersion: 1.0.0.0
+PackageLocale: en-US
+PackageName: AppInstaller Test Zip Installer
+ShortDescription: AppInstaller Test Zip Installer with exe
+Publisher: Microsoft Corporation
+Moniker: AICLITestZip
+License: Test
+Installers:
+    - Architecture: x86
+      InstallerUrl: https://ThisIsNotUsed
+      InstallerType: zip
+      InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+      NestedInstallerType: exe
+      NestedInstallerFiles:
+        - RelativeFilePath: ../../RelativeFilePath
+      InstallerSwitches:
+        Custom: /custom /scope=machine
+        SilentWithProgress: /silentwithprogress
+        Silent: /silence
+        Update: /update 
+ManifestType: singleton
+ManifestVersion: 1.3.0

src/AppInstallerCLITests/TestData/InstallFlowTest_Zip_MissingNestedInstaller.yaml

@@ -0,0 +1,21 @@
+PackageIdentifier: AppInstallerCliTest.TestZipInstaller
+PackageVersion: 1.0.0.0
+PackageLocale: en-US
+PackageName: AppInstaller Test Zip Installer
+ShortDescription: AppInstaller Test Zip Installer with exe
+Publisher: Microsoft Corporation
+Moniker: AICLITestZip
+License: Test
+Installers:
+    - Architecture: x86
+      InstallerUrl: https://ThisIsNotUsed
+      InstallerType: zip
+      InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+      NestedInstallerType: exe
+      InstallerSwitches:
+        Custom: /custom /scope=machine
+        SilentWithProgress: /silentwithprogress
+        Silent: /silence
+        Update: /update 
+ManifestType: singleton
+ManifestVersion: 1.3.0

src/AppInstallerCLITests/WorkFlow.cpp

@@ -1033,6 +1033,46 @@ TEST_CASE("InstallFlow_Zip_BadRelativePath", "[InstallFlow][workflow]")
     REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::NestedInstallerNotFound).get()) != std::string::npos);
 }
 
+TEST_CASE("InstallFlow_Zip_RelativePathContainsEscapeString", "[InstallFlow][workflow]")
+{
+    TestCommon::TempFile installResultPath("TestExeInstalled.txt");
+
+    std::ostringstream installOutput;
+    TestContext context{ installOutput, std::cin };
+    auto previousThreadGlobals = context.SetForCurrentThread();
+    context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Zip_InvalidRelativeFilePath.yaml").GetPath().u8string());
+
+    InstallCommand install({});
+    install.Execute(context);
+    INFO(installOutput.str());
+
+    REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_NESTEDINSTALLER_INVALID_PATH);
+
+    // Verify Installer was not called
+    REQUIRE(!std::filesystem::exists(installResultPath.GetPath()));
+    REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::InvalidRelativeFilePathToNestedInstaller).get()) != std::string::npos);
+}
+
+TEST_CASE("InstallFlow_Zip_MissingNestedInstaller", "[InstallFlow][workflow]")
+{
+    TestCommon::TempFile installResultPath("TestExeInstalled.txt");
+
+    std::ostringstream installOutput;
+    TestContext context{ installOutput, std::cin };
+    auto previousThreadGlobals = context.SetForCurrentThread();
+    context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Zip_MissingNestedInstaller.yaml").GetPath().u8string());
+
+    InstallCommand install({});
+    install.Execute(context);
+    INFO(installOutput.str());
+
+    REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_INVALID_MANIFEST);
+
+    // Verify Installer was not called
+    REQUIRE(!std::filesystem::exists(installResultPath.GetPath()));
+    REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::NestedInstallerNotSpecified).get()) != std::string::npos);
+}
+
 TEST_CASE("ExtractInstallerFromArchive_InvalidZip", "[InstallFlow][workflow]")
 {
     std::ostringstream installOutput;

src/AppInstallerCommonCore/Public/AppInstallerErrors.h

@@ -105,6 +105,7 @@
 #define APPINSTALLER_CLI_ERROR_BIND_WITH_EMBEDDED_NULL          ((HRESULT)0x8A15005A)
 #define APPINSTALLER_CLI_ERROR_NESTEDINSTALLER_NOT_FOUND        ((HRESULT)0x8A15005B)
 #define APPINSTALLER_CLI_ERROR_EXTRACT_ARCHIVE_FAILED           ((HRESULT)0x8A15005C)
+#define APPINSTALLER_CLI_ERROR_NESTEDINSTALLER_INVALID_PATH     ((HRESULT)0x8A15005D)
 
 // Install errors.
 #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE           ((HRESULT)0x8A150101)