Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate URLs map to unique hashes #5233

Merged
merged 9 commits into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -539,12 +539,18 @@
<CopyFileToFolders Include="TestData\Manifest-Bad-DuplicateReturnCode-SuccessCodes.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Bad-DuplicateSha256.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Bad-IdInvalid.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Bad-IdMissing.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSha256.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Bad-InstallersMissing.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
Expand Down Expand Up @@ -1092,4 +1098,4 @@
<Error Condition="!Exists('$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.props')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.props'))" />
<Error Condition="!Exists('$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.targets')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.targets'))" />
</Target>
</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Bad manifest. Different URLs point to two the same InstallerSha256
Id: microsoft.msixsdk
Name: MSIX SDK
Version: 1.7.32
Publisher: Microsoft
InstallerType: Msi
License: Test
Installers:
- Arch: x86
Url: https://ThisIsNotUsed
Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCD
Language: en-US
- Arch: x86
Url: https://ThisIsNotUsed2
Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCD
Language: es-MX
ManifestVersion: 0.1.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Bad manifest. Same URL points to two different InstallerSha256
PackageIdentifier: AppInstallerCliTest.TestPortableInstaller
PackageVersion: 1.0.0.0
PackageLocale: en-US
PackageName: AppInstaller Test Portable Exe
Publisher: Microsoft Corporation
AppMoniker: AICLITestPortable
License: Test
ProductCode: AppInstallerCliTest.TestPortableInstaller__TestSource
Installers:
- Architecture: x86
InstallerUrl: https://ThisIsNotUsed
InstallerType: portable
InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
- Architecture: x64
InstallerUrl: https://ThisIsNotUsed
InstallerType: portable
InstallerSha256: A111111111111111111111111111111111111111111111111111111111111111
ManifestType: singleton
ManifestVersion: 1.10.0
2 changes: 2 additions & 0 deletions src/AppInstallerCLITests/YamlManifest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,10 @@ TEST_CASE("ReadBadManifests", "[ManifestValidation]")
{ "Manifest-Bad-DuplicateKey-DifferentCase-lower.yaml", "Duplicate field found in the manifest." },
{ "Manifest-Bad-DuplicateReturnCode-ExpectedCodes.yaml", "Duplicate installer return code found." },
{ "Manifest-Bad-DuplicateReturnCode-SuccessCodes.yaml", "Duplicate installer return code found." },
{ "Manifest-Bad-DuplicateSha256.yaml", "Multiple Installer URLs found with the same InstallerSha256. Please ensure the accuracy of the URLs", true },
{ "Manifest-Bad-IdInvalid.yaml", "Failed to validate against schema associated with property name 'Id'" },
{ "Manifest-Bad-IdMissing.yaml", "Missing required property 'Id'" },
{ "Manifest-Bad-InconsistentSha256.yaml", "The values of InstallerSha256 do not match for all instances of the same InstallerUrl" },
{ "Manifest-Bad-InstallersMissing.yaml", "Missing required property 'Installers'" },
{ "Manifest-Bad-InstallerTypeExe-NoSilent.yaml", "Silent and SilentWithProgress switches are not specified for InstallerType exe.", true },
{ "Manifest-Bad-InstallerTypeExe-NoSilentRoot.yaml", "Silent and SilentWithProgress switches are not specified for InstallerType exe.", true },
Expand Down
31 changes: 31 additions & 0 deletions src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace AppInstaller::Manifest
{ AppInstaller::Manifest::ManifestError::FieldNotSupported, "Field is not supported."sv },
{ AppInstaller::Manifest::ManifestError::FieldValueNotSupported, "Field value is not supported."sv },
{ AppInstaller::Manifest::ManifestError::DuplicateInstallerEntry, "Duplicate installer entry found."sv },
{ AppInstaller::Manifest::ManifestError::DuplicateInstallerHash, "Multiple Installer URLs found with the same InstallerSha256. Please ensure the accuracy of the URLs"sv },
{ AppInstaller::Manifest::ManifestError::InstallerTypeDoesNotSupportPackageFamilyName, "The specified installer type does not support PackageFamilyName."sv },
{ AppInstaller::Manifest::ManifestError::InstallerTypeDoesNotSupportProductCode, "The specified installer type does not support ProductCode."sv },
{ AppInstaller::Manifest::ManifestError::InstallerTypeDoesNotWriteAppsAndFeaturesEntry, "The specified installer type does not write to Apps and Features entry."sv },
Expand All @@ -37,6 +38,7 @@ namespace AppInstaller::Manifest
{ AppInstaller::Manifest::ManifestError::DuplicateMultiFileManifestType, "The multi file manifest should contain only one file with the particular ManifestType."sv },
{ AppInstaller::Manifest::ManifestError::DuplicateMultiFileManifestLocale, "The multi file manifest contains duplicate PackageLocale."sv },
{ AppInstaller::Manifest::ManifestError::UnsupportedMultiFileManifestType, "The multi file manifest should not contain file with the particular ManifestType."sv },
{ AppInstaller::Manifest::ManifestError::InconsistentInstallerHash, "The values of InstallerSha256 do not match for all instances of the same InstallerUrl"sv },
{ AppInstaller::Manifest::ManifestError::InconsistentMultiFileManifestDefaultLocale, "DefaultLocale value in version manifest does not match PackageLocale value in defaultLocale manifest."sv },
{ AppInstaller::Manifest::ManifestError::FieldFailedToProcess, "Failed to process field."sv },
{ AppInstaller::Manifest::ManifestError::InvalidBcp47Value, "The locale value is not a well formed bcp47 language tag."sv },
Expand Down Expand Up @@ -141,6 +143,10 @@ namespace AppInstaller::Manifest
std::set<ManifestInstaller, decltype(installerCmp)> installerSet(installerCmp);
bool duplicateInstallerFound = false;

// Set up maps for checking uniqueness across hash <-> url pairs
std::unordered_map<std::string, std::string> urlToChecksum;
std::unordered_map<std::string, std::string> checksumToUrl;

// Validate installers
for (auto const& installer : manifest.Installers)
{
Expand Down Expand Up @@ -223,6 +229,31 @@ namespace AppInstaller::Manifest
{
resultErrors.emplace_back(ManifestError::FieldNotSupported, "ProductId");
}

// Ensure that each URL has a one to one mapping with a Sha256 and
// warn if a Sha256 has a one to many mapping with a URL
if (!installer.Url.empty() && !installer.Sha256.empty())
{
std::string checksum = Utility::SHA256::ConvertToString(installer.Sha256);
std::string url = installer.Url;

auto [urlIterator, urlInserted] = urlToChecksum.try_emplace(url, checksum);
auto [checksumIterator, checksumInserted] = checksumToUrl.try_emplace(checksum, url);

if (!urlInserted && urlIterator->second != checksum)
{
// If the URL was not inserted, and the value in the map does not match the current Sha256, then
// a single URL corresponds to multiple SHA256 and an error should be thrown
resultErrors.emplace_back(ManifestError::InconsistentInstallerHash, "InstallerUrl", url);
}

if (!checksumInserted && checksumIterator->second != url)
{
// If the SHA256 was not inserted, and the value in the map does not match the current URL, then
// a single SHA256 corresponds to multiple URLS and a warning should be thrown
resultErrors.emplace_back(ManifestError::DuplicateInstallerHash, "InstallerSha256", checksum, ValidationError::Level::Warning);
}
}
}

if (installer.EffectiveInstallerType() == InstallerTypeEnum::Exe &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace AppInstaller::Manifest
WINGET_DEFINE_RESOURCE_STRINGID(DuplicateMultiFileManifestLocale);
WINGET_DEFINE_RESOURCE_STRINGID(DuplicateMultiFileManifestType);
WINGET_DEFINE_RESOURCE_STRINGID(DuplicateInstallerEntry);
WINGET_DEFINE_RESOURCE_STRINGID(DuplicateInstallerHash);
WINGET_DEFINE_RESOURCE_STRINGID(DuplicateReturnCodeEntry);
WINGET_DEFINE_RESOURCE_STRINGID(ExceededAppsAndFeaturesEntryLimit);
WINGET_DEFINE_RESOURCE_STRINGID(ExceededCommandsLimit);
Expand All @@ -42,6 +43,7 @@ namespace AppInstaller::Manifest
WINGET_DEFINE_RESOURCE_STRINGID(FieldValueNotSupported);
WINGET_DEFINE_RESOURCE_STRINGID(FoundDependencyLoop);
WINGET_DEFINE_RESOURCE_STRINGID(IncompleteMultiFileManifest);
WINGET_DEFINE_RESOURCE_STRINGID(InconsistentInstallerHash);
WINGET_DEFINE_RESOURCE_STRINGID(InconsistentMultiFileManifestDefaultLocale);
WINGET_DEFINE_RESOURCE_STRINGID(InconsistentMultiFileManifestFieldValue);
WINGET_DEFINE_RESOURCE_STRINGID(InstallerFailedToProcess);
Expand Down
Loading