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(InvalidPathToNestedInstaller);
         WINGET_DEFINE_RESOURCE_STRINGID(LicenseAgreement);
         WINGET_DEFINE_RESOURCE_STRINGID(Links);
         WINGET_DEFINE_RESOURCE_STRINGID(ListCommandLongDescription);
@@ -187,9 +188,11 @@ namespace AppInstaller::CLI::Resource
         WINGET_DEFINE_RESOURCE_STRINGID(MSStoreInstallTryGetEntitlement);
         WINGET_DEFINE_RESOURCE_STRINGID(MSStoreStoreClientBlocked);
         WINGET_DEFINE_RESOURCE_STRINGID(MultipleInstalledPackagesFound);
+        WINGET_DEFINE_RESOURCE_STRINGID(MultipleNonPortableNestedInstallersSpecified);
         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

@@ -3,6 +3,9 @@
 #include "pch.h"
 #include "ArchiveFlow.h"
 #include "winget/Archive.h"
+#include "winget/Filesystem.h"
+
+using namespace AppInstaller::Manifest;
 
 namespace AppInstaller::CLI::Workflow
 {
@@ -32,19 +35,24 @@ 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);
         }
 
         const auto& installerPath = context.Get<Execution::Data::InstallerPath>();
         const auto& installerParentPath = installerPath.parent_path();
-
         const auto& relativeFilePath = ConvertToUTF16(installer.NestedInstallerFiles[0].RelativeFilePath);
 
-        std::filesystem::path nestedInstallerPath = installerParentPath / relativeFilePath;
+        const std::filesystem::path& nestedInstallerPath = installerParentPath / relativeFilePath;
 
-        if (!std::filesystem::exists(nestedInstallerPath))
+        if (Filesystem::PathEscapesBaseDirectory(nestedInstallerPath, installerParentPath))
+        {
+            AICLI_LOG(CLI, Error, << "Path points to a location outside of the install directory: " << nestedInstallerPath);
+            context.Reporter.Error() << Resource::String::InvalidPathToNestedInstaller << std::endl;
+            AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NESTEDINSTALLER_INVALID_PATH);
+        }
+        else if (!std::filesystem::exists(nestedInstallerPath))
         {
             AICLI_LOG(CLI, Error, << "Unable to locate nested installer at: " << nestedInstallerPath);
             context.Reporter.Error() << Resource::String::NestedInstallerNotFound << ' ' << nestedInstallerPath << std::endl;
@@ -56,4 +64,27 @@ namespace AppInstaller::CLI::Workflow
             context.Add<Execution::Data::InstallerPath>(nestedInstallerPath);
         }
     }
+
+    void EnsureValidNestedInstallerMetadataForArchiveInstall(Execution::Context& context)
+    {
+        auto installer = context.Get<Execution::Data::Installer>().value();
+
+        if (IsArchiveType(installer.InstallerType))
+        {
+            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);
+            }
+
+            if (installer.NestedInstallerType != InstallerTypeEnum::Portable && nestedInstallerFiles.size() != 1)
+            {
+                AICLI_LOG(CLI, Error, << "Multiple nested installers specified for non-portable nested installerType");
+                context.Reporter.Error() << Resource::String::MultipleNonPortableNestedInstallersSpecified << std::endl;
+                AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INVALID_MANIFEST);
+            }
+        }
+    }
 }

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/AppInstallerCLIE2ETests/InstallCommand.cs

@@ -281,6 +281,14 @@ public void InstallZipWithExe()
             Assert.True(VerifyTestExeInstalled(installDir, "/execustom"));
         }
 
+        [Test]
+        public void InstallZipWithInvalidRelativeFilePath()
+        {
+            var result = TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestZipInvalidRelativePath");
+            Assert.AreNotEqual(Constants.ErrorCode.S_OK, result.ExitCode);
+            Assert.True(result.StdOut.Contains("Invalid relative file path to the nested installer; path points to a location outside of the install directory"));
+        }
+
         [Test]
         public void InstallZipWithMsi()
         {

src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Exe_InvalidRelativeFilePath.yaml

@@ -0,0 +1,18 @@
+PackageIdentifier: AppInstallerTest.TestZipInvalidRelativePath
+PackageVersion: 1.0.0.0
+PackageName: TestZipInvalidRelativePath
+PackageLocale: en-US
+Publisher: AppInstallerTest
+License: Test
+ShortDescription: E2E test for installing a zip with a bad relative file path.
+Installers:
+  - Architecture: x64
+    InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestZipInstaller/AppInstallerTestZipInstaller.zip
+    InstallerType: zip
+    ProductCode: '{A499DD5E-8DC5-4AD2-911A-BCD0263295E9}'
+    InstallerSha256: <ZIPHASH>
+    NestedInstallerType: exe
+    NestedInstallerFiles:
+      - RelativeFilePath: ../../AppInstallerTestExeInstaller.exe
+ManifestType: singleton
+ManifestVersion: 1.3.0

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

@@ -1370,4 +1370,13 @@ 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="InvalidPathToNestedInstaller" xml:space="preserve">
+    <value>Invalid relative file path to the nested installer; path points to a location outside of the install directory</value>
+  </data>
+  <data name="NestedInstallerNotSpecified" xml:space="preserve">
+    <value>No nested installers specified for this package</value>
+  </data>
+  <data name="MultipleNonPortableNestedInstallersSpecified" xml:space="preserve">
+    <value>Only one non-portable nested installer can be specified for an archive installer</value>
+  </data>
 </root>

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj

@@ -196,6 +196,7 @@
     <ClCompile Include="Dependencies.cpp" />
     <ClCompile Include="Downloader.cpp" />
     <ClCompile Include="ExperimentalFeature.cpp" />
+    <ClCompile Include="Filesystem.cpp" />
     <ClCompile Include="GroupPolicy.cpp" />
     <ClCompile Include="HashCommand.cpp" />
     <ClCompile Include="HttpClientHelper.cpp" />
@@ -286,6 +287,12 @@
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\InstallFlowTest_ZipWithExe.yaml">
       <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_MissingNestedInstaller.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_MultipleNonPortableNestedInstallers.yaml">
+      <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>    
     <CopyFileToFolders Include="TestData\ImportFile-Bad-Invalid.json">
       <DeploymentContent>true</DeploymentContent>
@@ -416,6 +423,9 @@
     <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypePortable-InvalidCommands.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-MissingRelativeFilePath.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters

@@ -212,6 +212,9 @@
     <ClCompile Include="Archive.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="Filesystem.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="PropertySheet.props" />
@@ -297,6 +300,9 @@
     <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypePortable-InvalidCommands.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-MissingRelativeFilePath.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
@@ -509,6 +515,12 @@
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\InstallFlowTest_ZipWithExe.yaml">
       <Filter>TestData</Filter>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_MissingNestedInstaller.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_Zip_MultipleNonPortableNestedInstallers.yaml">
+      <Filter>TestData</Filter>
     </CopyFileToFolders>    
     <CopyFileToFolders Include="TestData\InstallerArgTest_Msi_WithSwitches.yaml">
       <Filter>TestData</Filter>

src/AppInstallerCLITests/Filesystem.cpp

@@ -0,0 +1,31 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+#include "pch.h"
+#include "TestCommon.h"
+#include <winget/Filesystem.h>
+#include <AppInstallerStrings.h>
+
+using namespace AppInstaller::Utility;
+using namespace AppInstaller::Filesystem;
+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));
+}

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/TestData/InstallFlowTest_Zip_MultipleNonPortableNestedInstallers.yaml

@@ -0,0 +1,19 @@
+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: installerOne.exe
+        - RelativeFilePath: installerTwo.exe
+ManifestType: singleton
+ManifestVersion: 1.3.0

src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml

@@ -0,0 +1,20 @@
+# Bad manifest. A nested installer file must have a RelativeFilePath specified.
+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 without RelativeFilePath specified
+Scope: User
+Installers:
+  - Architecture: x64
+    InstallerUrl: https://ThisIsNotUsed
+    InstallerType: zip
+    InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+    NestedInstallerType: exe
+    NestedInstallerFiles:
+      - RelativeFilePath: ../../relativeFilePath
+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_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("InstallFlow_Zip_MultipleNonPortableNestedInstallers", "[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_MultipleNonPortableNestedInstallers.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::MultipleNonPortableNestedInstallersSpecified).get()) != std::string::npos);
+}
+
 TEST_CASE("ExtractInstallerFromArchive_InvalidZip", "[InstallFlow][workflow]")
 {
     std::ostringstream installOutput;

src/AppInstallerCLITests/YamlManifest.cpp

@@ -257,6 +257,7 @@ TEST_CASE("ReadBadManifests", "[ManifestValidation]")
         { "Manifest-Bad-InstallerTypePortable-InvalidAppsAndFeatures.yaml", "Only zero or one entry for Apps and Features may be specified for InstallerType portable." },
         { "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-InvalidRelativeFilePath.yaml", "Relative file path must not point to a location outside of archive directory" },
         { "Manifest-Bad-InstallerTypeZip-MissingRelativeFilePath.yaml", "Required field missing. Field: RelativeFilePath" },
         { "Manifest-Bad-InstallerTypeZip-MultipleNestedInstallers.yaml", "Only one entry for NestedInstallerFiles can be specified for non-portable InstallerTypes." },
         { "Manifest-Bad-InstallerTypeZip-NoNestedInstallerFile.yaml", "Required field missing. Field: NestedInstallerFiles" },

src/AppInstallerCommonCore/Errors.cpp

@@ -224,6 +224,8 @@ namespace AppInstaller
                 return "Embedded null characters are disallowed for SQLite";
             case APPINSTALLER_CLI_ERROR_NESTEDINSTALLER_NOT_FOUND:
                 return "Failed to find the nested installer in the archive.";
+            case APPINSTALLER_CLI_ERROR_NESTEDINSTALLER_INVALID_PATH:
+                return "Invalid relative file path to nested installer provided.";
             default:
                 return "Unknown Error Code";
             }