winget-cli

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | README | LICENSE

commit 34f22ce6c1e5e05204708e272513161faf6d846b
parent 8abc3252b4cf9109e370eff2bd9528e12b4e03ed
Author: Ryan Fu <69221034+ryfu-msft@users.noreply.github.com>
Date:   Wed, 20 Jul 2022 13:57:50 -0700

Prevent access to parent directories from relativeFilePath for archive install (#2342)


Diffstat:
Msrc/AppInstallerCLICore/Resources.h | 3+++
Msrc/AppInstallerCLICore/Workflows/ArchiveFlow.cpp | 39+++++++++++++++++++++++++++++++++++----
Msrc/AppInstallerCLICore/Workflows/ArchiveFlow.h | 6++++++
Msrc/AppInstallerCLICore/Workflows/InstallFlow.cpp | 3++-
Msrc/AppInstallerCLIE2ETests/InstallCommand.cs | 8++++++++
Asrc/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Exe_InvalidRelativeFilePath.yaml | 19+++++++++++++++++++
Msrc/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw | 9+++++++++
Msrc/AppInstallerCLITests/AppInstallerCLITests.vcxproj | 10++++++++++
Msrc/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters | 12++++++++++++
Asrc/AppInstallerCLITests/Filesystem.cpp | 32++++++++++++++++++++++++++++++++
Asrc/AppInstallerCLITests/TestData/InstallFlowTest_Zip_MissingNestedInstaller.yaml | 22++++++++++++++++++++++
Asrc/AppInstallerCLITests/TestData/InstallFlowTest_Zip_MultipleNonPortableNestedInstallers.yaml | 20++++++++++++++++++++
Asrc/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml | 21+++++++++++++++++++++
Msrc/AppInstallerCLITests/WorkFlow.cpp | 40++++++++++++++++++++++++++++++++++++++++
Msrc/AppInstallerCLITests/YamlManifest.cpp | 1+
Msrc/AppInstallerCommonCore/Errors.cpp | 2++
Msrc/AppInstallerCommonCore/Filesystem.cpp | 8++++++++
Msrc/AppInstallerCommonCore/Manifest/ManifestValidation.cpp | 10++++++++++
Msrc/AppInstallerCommonCore/Public/AppInstallerErrors.h | 1+
Msrc/AppInstallerCommonCore/Public/winget/Filesystem.h | 3+++
Msrc/AppInstallerCommonCore/Public/winget/ManifestValidation.h | 1+
21 files changed, 265 insertions(+), 5 deletions(-)

diff --git a/src/AppInstallerCLICore/Resources.h b/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); diff --git a/src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp b/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); + } + } + } } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/ArchiveFlow.h b/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); } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/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 diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -282,6 +282,14 @@ namespace AppInstallerCLIE2ETests } [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() { if (string.IsNullOrEmpty(TestCommon.MsiInstallerPath)) diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Exe_InvalidRelativeFilePath.yaml b/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+ \ No newline at end of file diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/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> \ No newline at end of file diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/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" /> @@ -287,6 +288,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> @@ -417,6 +424,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> diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -215,6 +215,9 @@ <ClCompile Include="Archive.cpp"> <Filter>Source Files\Common</Filter> </ClCompile> + <ClCompile Include="Filesystem.cpp"> + <Filter>Source Files</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> @@ -300,6 +303,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> @@ -512,6 +518,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> diff --git a/src/AppInstallerCLITests/Filesystem.cpp b/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)); +}+ \ No newline at end of file diff --git a/src/AppInstallerCLITests/TestData/InstallFlowTest_Zip_MissingNestedInstaller.yaml b/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+ \ No newline at end of file diff --git a/src/AppInstallerCLITests/TestData/InstallFlowTest_Zip_MultipleNonPortableNestedInstallers.yaml b/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+ \ No newline at end of file diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-InvalidRelativeFilePath.yaml b/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+ \ No newline at end of file diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/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; diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/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" }, diff --git a/src/AppInstallerCommonCore/Errors.cpp b/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"; } diff --git a/src/AppInstallerCommonCore/Filesystem.cpp b/src/AppInstallerCommonCore/Filesystem.cpp @@ -62,6 +62,14 @@ namespace AppInstaller::Filesystem return (GetVolumeInformationFlags(path) & FILE_SUPPORTS_REPARSE_POINTS) != 0; } + bool PathEscapesBaseDirectory(const std::filesystem::path& target, const std::filesystem::path& base) + { + 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()); + } + // Complicated rename algorithm due to somewhat arbitrary failures. // 1. First, try to rename. // 2. Then, create an empty file for the target, and attempt to rename. diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -6,6 +6,7 @@ #include "winget/MsixManifest.h" #include "winget/ManifestValidation.h" #include "winget/Locale.h" +#include "winget/Filesystem.h" namespace AppInstaller::Manifest { @@ -196,6 +197,15 @@ namespace AppInstaller::Manifest { resultErrors.emplace_back(ManifestError::RequiredFieldMissing, "RelativeFilePath"); } + else + { + const std::filesystem::path& basePath = std::filesystem::current_path(); + const std::filesystem::path& fullPath = basePath / ConvertToUTF16(nestedInstallerFile.RelativeFilePath); + if (AppInstaller::Filesystem::PathEscapesBaseDirectory(fullPath, basePath)) + { + resultErrors.emplace_back(ManifestError::RelativeFilePathEscapesDirectory, "RelativeFilePath"); + } + } } } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/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) diff --git a/src/AppInstallerCommonCore/Public/winget/Filesystem.h b/src/AppInstallerCommonCore/Public/winget/Filesystem.h @@ -14,6 +14,9 @@ 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); + // Renames the file to a new path. void RenameFile(const std::filesystem::path& from, const std::filesystem::path& to); } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h @@ -59,6 +59,7 @@ namespace AppInstaller::Manifest const char* const ArpVersionOverlapWithIndex = "DisplayVersion declared in the manifest has overlap with existing DisplayVersion range in the index. Existing DisplayVersion range in index: "; const char* const ArpVersionValidationInternalError = "Internal error while validating DisplayVersion against index."; const char* const ExceededNestedInstallerFilesLimit = "Only one entry for NestedInstallerFiles can be specified for non-portable InstallerTypes."; + const char* const RelativeFilePathEscapesDirectory = "Relative file path must not point to a location outside of archive directory"; } struct ValidationError