From 94a597ab1b4afc838c72de889ca1a8ca061dbf9f Mon Sep 17 00:00:00 2001 From: Amir El Bawab <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 11 Nov 2026 11:58:25 -0800 Subject: [PATCH 1/2] Verify ZIP symlink data only after its bounds check in PureLib --- src/PureLib/pure/pure.h | 11 +++++++---- src/PureLib/readme.md | 7 +++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/PureLib/pure/pure.h b/src/PureLib/pure/pure.h index 6f7e26fabc..facab9be83 100644 --- a/src/PureLib/pure/pure.h +++ b/src/PureLib/pure/pure.h @@ -1,3 +1,5 @@ +// This file has been modified for WinGet. See src/PureLib/readme.md for the local changes. + // pure.h - C99 interface: #ifndef PURE_H @@ -1658,10 +1660,6 @@ int pure_zip_meta( int error = pure_zip_diff_cdh_lfh(&cdh, &lfh); if (error) return error; } - { - int error = pure_zip_verify_symlink(&cdh, &lfh, buffer); - if (error) return error; - } assert(lfh.length >= PURE_ZIP_LFH_MIN); lfh_offset += lfh.length; // File Data (compressed or uncompressed): @@ -1670,6 +1668,11 @@ int pure_zip_meta( } lfh_offset += cdh.compressed_size; if (lfh_offset > size) return PURE_E_ZIP_LFH_DATA_OVERFLOW; + // Verify symlink data only after checking that it is within the buffer: + { + int error = pure_zip_verify_symlink(&cdh, &lfh, buffer); + if (error) return error; + } // Data Descriptor Record (optional): if (lfh.general_purpose_bit_flag & (1 << 3)) { { diff --git a/src/PureLib/readme.md b/src/PureLib/readme.md index a53563aad6..8934c3bfd7 100644 --- a/src/PureLib/readme.md +++ b/src/PureLib/readme.md @@ -7,6 +7,13 @@ It is created using git subtree command: The PureLib.vcxitems is created to make pure code compiled as part of the WinGet solution. +#### Local changes + +Keep these changes to pure.h when updating the subtree: + +- It includes `zlib.h` instead of `zlib/zlib.h`, since zlib comes from vcpkg. +- `pure_zip_meta` verifies symlink data only after checking that the data is within the buffer. + #### Steps used to create the VS project files. 1. VS Create project from existing code wizard to create a Shared Items template. From c1a172b315f3e994c36bcda2ba6c941a28c364d0 Mon Sep 17 00:00:00 2001 From: Amir El Bawab <104940545+AmelBawa-msft@users.noreply.github.com> Date: Wed, 11 Nov 2026 13:49:36 -0800 Subject: [PATCH 2/2] Add PureLib regression tests for ZIP symlink bounds checking --- src/AppInstallerCLITests/Archive.cpp | 97 ++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/src/AppInstallerCLITests/Archive.cpp b/src/AppInstallerCLITests/Archive.cpp index 2aafb57e59..2b756242d6 100644 --- a/src/AppInstallerCLITests/Archive.cpp +++ b/src/AppInstallerCLITests/Archive.cpp @@ -4,10 +4,90 @@ #include "TestCommon.h" #include +// Defined in PureLib, which is compiled into AppInstallerCommonCore. +int pure_zip(const uint8_t* buffer, const uint64_t size, const uint64_t flags); +const char* pure_error_code(const int error); + using namespace AppInstaller::Archive; using namespace TestCommon; constexpr std::string_view s_ZipFile = "TestZip.zip"; +constexpr std::string_view s_SymlinkName = "link"; +constexpr std::string_view s_SymlinkTarget = "target.txt"; + +namespace +{ + void AppendUInt16(std::vector& zip, uint16_t value) + { + zip.push_back(static_cast(value)); + zip.push_back(static_cast(value >> 8)); + } + + void AppendUInt32(std::vector& zip, uint32_t value) + { + AppendUInt16(zip, static_cast(value)); + AppendUInt16(zip, static_cast(value >> 16)); + } + + void AppendString(std::vector& zip, std::string_view value) + { + zip.insert(zip.end(), value.begin(), value.end()); + } + + // Fields shared by the local file header and the central directory header. + void AppendEntryFields(std::vector& zip, uint32_t dataSize) + { + AppendUInt16(zip, 20); // Version needed + AppendUInt16(zip, 0); // Flags + AppendUInt16(zip, 0); // Stored + AppendUInt16(zip, 0); // Time + AppendUInt16(zip, 0x5C21); // Date + AppendUInt32(zip, 0x00A4BA49); // CRC-32 of s_SymlinkTarget + AppendUInt32(zip, dataSize); // Compressed size + AppendUInt32(zip, dataSize); // Uncompressed size + AppendUInt16(zip, static_cast(s_SymlinkName.size())); + AppendUInt16(zip, 0); // Extra length + } + + // Creates a ZIP with a stored Unix symlink whose headers declare dataSize bytes of target data. + std::vector CreateSymlinkZip(uint32_t dataSize) + { + std::vector zip; + + AppendUInt32(zip, 0x04034B50); // Local file header + AppendEntryFields(zip, dataSize); + AppendString(zip, s_SymlinkName); + AppendString(zip, s_SymlinkTarget); + + uint32_t centralDirectoryOffset = static_cast(zip.size()); + AppendUInt32(zip, 0x02014B50); // Central directory header + AppendUInt16(zip, 0x0314); // Made by Unix + AppendEntryFields(zip, dataSize); + AppendUInt16(zip, 0); // Comment length + AppendUInt16(zip, 0); // Disk + AppendUInt16(zip, 0); // Internal attributes + AppendUInt32(zip, 0xA1FF0000); // Unix symlink mode + AppendUInt32(zip, 0); // Local header offset + AppendString(zip, s_SymlinkName); + + uint32_t centralDirectorySize = static_cast(zip.size()) - centralDirectoryOffset; + AppendUInt32(zip, 0x06054B50); // End of central directory + AppendUInt16(zip, 0); // Disk + AppendUInt16(zip, 0); // Central directory disk + AppendUInt16(zip, 1); // Entries on disk + AppendUInt16(zip, 1); // Total entries + AppendUInt32(zip, centralDirectorySize); + AppendUInt32(zip, centralDirectoryOffset); + AppendUInt16(zip, 0); // Comment length + + return zip; + } + + std::string_view GetScanResult(const std::vector& zip) + { + return pure_error_code(pure_zip(zip.data(), zip.size(), 0)); + } +} TEST_CASE("Extract_ZipArchive", "[archive]") { @@ -32,3 +112,20 @@ TEST_CASE("Scan_ZipArchive", "[archive]") bool result = ScanZipFile(testZipPath); REQUIRE(result); } + +TEST_CASE("Scan_ZipArchive_Symlink", "[archive]") +{ + auto zip = CreateSymlinkZip(static_cast(s_SymlinkTarget.size())); + REQUIRE(GetScanResult(zip) == "PURE_E_OK"); +} + +TEST_CASE("Scan_ZipArchive_SymlinkDataPastEnd", "[archive]") +{ + auto zip = CreateSymlinkZip(static_cast(s_SymlinkTarget.size())); + + // Declare one byte more than the buffer holds after the 30-byte local file header and name. + auto dataSize = zip.size() - 30 - s_SymlinkName.size() + 1; + zip = CreateSymlinkZip(static_cast(dataSize)); + + REQUIRE(GetScanResult(zip) == "PURE_E_ZIP_LFH_DATA_OVERFLOW"); +}