tp: support stored zip entries with data descriptors Zips written to a non-seekable stream (e.g. GitHub Actions artifacts uploaded without compression) set general purpose flag bit 3 and leave the sizes in the local header as zero, putting them in a trailing data descriptor instead. We supported this for deflated entries, where the deflate stream marks its own end, but rejected it for stored entries. For stored entries, find the end of the payload by scanning for the data descriptor. As the payload can contain the descriptor signature, a candidate is accepted only if its sizes match the bytes preceding it, it is followed by a file header or the central directory and, if zlib is available, its CRC32 matches the payload.
diff --git a/src/trace_processor/util/zip_reader.cc b/src/trace_processor/util/zip_reader.cc index 325edf3..45636c9 100644 --- a/src/trace_processor/util/zip_reader.cc +++ b/src/trace_processor/util/zip_reader.cc
@@ -27,6 +27,7 @@ #include <vector> #include "perfetto/base/build_config.h" +#include "perfetto/base/compiler.h" #include "perfetto/base/logging.h" #include "perfetto/base/status.h" #include "perfetto/base/time.h" @@ -91,6 +92,13 @@ return res; } +#if PERFETTO_BUILDFLAG(PERFETTO_ZLIB) +uint32_t ComputeCrc32(const uint8_t* data, size_t len) { + return static_cast<uint32_t>(::crc32( + 0u, reinterpret_cast<const ::Bytef*>(data), static_cast<::uInt>(len))); +} +#endif + } // namespace ZipReader::ZipReader() = default; @@ -201,12 +209,6 @@ "deflate and no compression are supported.", reader_.start_offset(), cur_.hdr.compression); } - if (cur_.hdr.flags & kDataDescriptor && cur_.hdr.compression != kDeflate) { - return base::ErrStatus( - "Unsupported compression type at offset 0x%zx. type=%x. Only " - "deflate supported for ZIPs compressed in a streaming fashion.", - reader_.start_offset(), cur_.hdr.compression); - } cur_.parse_state = FileParseState::kFilename; return base::OkStatus(); } @@ -307,7 +309,9 @@ // Build up the compressed payload if (cur_.hdr.flags & kDataDescriptor) { if (!cur_.compressed) { - ASSIGN_OR_RETURN(auto compressed, TryParseUnsizedCompressedData()); + ASSIGN_OR_RETURN(auto compressed, cur_.hdr.compression == kDeflate + ? TryParseUnsizedCompressedData() + : TryParseUnsizedStoredData()); if (!compressed) { return base::OkStatus(); } @@ -402,6 +406,79 @@ return {std::move(raw_compressed)}; } +base::StatusOr<std::optional<TraceBlobView>> +ZipReader::TryParseUnsizedStoredData() { + PERFETTO_CHECK(cur_.hdr.compression == kNoCompression); + + // Unlike deflate, a stored payload has no end marker of its own. The only + // way to find its end is to look for the data descriptor that follows it. + // The payload is arbitrary data and can contain the descriptor signature, so + // a candidate is accepted only if: + // 1. Its sizes match the number of bytes preceding it (for stored entries, + // compressed == uncompressed size). + // 2. It is followed by the next file header or the central directory, as + // is the case in any valid zip. + // 3. Its CRC32 matches the payload (only if zlib is available). + uint32_t desc_size = + cur_.is_zip64 ? kZip64DataDescriptorSize : kDataDescriptorSize; + // The descriptor plus the signature of whatever follows it. + uint32_t lookahead = desc_size + sizeof(uint32_t); + size_t scan_start = reader_.start_offset() + cur_.unsized_scan_offset; + size_t avail = reader_.end_offset() - scan_start; + if (avail < lookahead) { + return {std::nullopt}; + } + auto slice = reader_.SliceOff(scan_start, avail); + PERFETTO_CHECK(slice); + + const uint8_t* data = slice->data(); + size_t last = avail - lookahead; + for (size_t i = 0; i <= last; ++i) { + const void* p = memchr(data + i, kDataDescriptorSig & 0xff, last - i + 1); + if (!p) { + break; + } + i = static_cast<size_t>(static_cast<const uint8_t*>(p) - data); + const uint8_t* desc_it = data + i; + if (ReadAndAdvance<uint32_t>(&desc_it) != kDataDescriptorSig) { + continue; + } + auto crc = ReadAndAdvance<uint32_t>(&desc_it); + uint64_t csize; + uint64_t usize; + if (cur_.is_zip64) { + csize = ReadAndAdvance<uint64_t>(&desc_it); + usize = ReadAndAdvance<uint64_t>(&desc_it); + } else { + csize = ReadAndAdvance<uint32_t>(&desc_it); + usize = ReadAndAdvance<uint32_t>(&desc_it); + } + size_t payload_size = cur_.unsized_scan_offset + i; + if (csize != payload_size || usize != payload_size) { + continue; + } + auto next_sig = ReadAndAdvance<uint32_t>(&desc_it); + if (next_sig != kFileHeaderSig && next_sig != kCentralDirectorySig) { + continue; + } + auto raw = reader_.SliceOff(reader_.start_offset(), payload_size); + PERFETTO_CHECK(raw); +#if PERFETTO_BUILDFLAG(PERFETTO_ZLIB) + if (ComputeCrc32(raw->data(), raw->size()) != crc) { + continue; + } +#else + base::ignore_result(crc); +#endif + PERFETTO_CHECK(reader_.PopFrontBytes(payload_size)); + return {std::move(raw)}; + } + // No descriptor in the bytes seen so far. Keep the last |lookahead - 1| + // bytes around for the next round as a descriptor might straddle them. + cur_.unsized_scan_offset += last + 1; + return {std::nullopt}; +} + ZipFile* ZipReader::Find(const std::string& path) { for (ZipFile& zf : files_) { if (zf.name() == path) @@ -444,9 +521,7 @@ out_data->resize(dec_res.bytes_written); #if PERFETTO_BUILDFLAG(PERFETTO_ZLIB) - const auto* crc_data = reinterpret_cast<const ::Bytef*>(out_data->data()); - auto crc_len = static_cast<::uInt>(out_data->size()); - auto actual_crc32 = static_cast<uint32_t>(::crc32(0u, crc_data, crc_len)); + uint32_t actual_crc32 = ComputeCrc32(out_data->data(), out_data->size()); if (actual_crc32 != hdr_.checksum) { return base::ErrStatus( "Zip CRC32 failure on %s (actual: %x, expected: %x) (ERR:tp-corrupt)",
diff --git a/src/trace_processor/util/zip_reader.h b/src/trace_processor/util/zip_reader.h index 4601691..0d515ca 100644 --- a/src/trace_processor/util/zip_reader.h +++ b/src/trace_processor/util/zip_reader.h
@@ -185,6 +185,9 @@ // Used to track the number of bytes fed into the decompressor when we don't // know the compressed size upfront. size_t decompressor_bytes_fed = 0; + // Used to track how far we have scanned for the data descriptor of a + // stored file when we don't know its size upfront. + size_t unsized_scan_offset = 0; // Held by unique_ptr, not value, as Decompressor isn't movable but // FileParseState needs to be in order to reset to a fresh instance for the // next file. @@ -201,6 +204,7 @@ base::Status TrySkipBytes(); base::Status TryParseCompressedData(); base::StatusOr<std::optional<TraceBlobView>> TryParseUnsizedCompressedData(); + base::StatusOr<std::optional<TraceBlobView>> TryParseUnsizedStoredData(); FileParseState cur_; std::vector<ZipFile> files_;
diff --git a/src/trace_processor/util/zip_reader_unittest.cc b/src/trace_processor/util/zip_reader_unittest.cc index c9a72c8..8d139de 100644 --- a/src/trace_processor/util/zip_reader_unittest.cc +++ b/src/trace_processor/util/zip_reader_unittest.cc
@@ -142,6 +142,64 @@ 0x00, 0x00, }; +// Zip archives written to a non-seekable stream with no compression: both +// stored entries use general-purpose flag bit 3 (data descriptor), so the +// local header sizes are zero and the real sizes live in a trailing data +// descriptor. The payload of "a" embeds a fake data descriptor (at offset 4, +// with sizes = 4) which the reader must not mistake for the real one. +// b content: "baz\n" +// +// The fake descriptor has a valid CRC but isn't followed by a header. +// a content: "foo " + fake descriptor + " bar\n" +const uint8_t kTestZipStoredDataDescriptorBadNextSig[] = { + 0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x08, 0x00, 0x00, 0x00, 0x6a, 0x85, + 0xf9, 0x54, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x61, 0x66, 0x6f, 0x6f, 0x20, 0x50, + 0x4b, 0x07, 0x08, 0x7e, 0xac, 0x89, 0xa5, 0x04, 0x00, 0x00, 0x00, 0x04, + 0x00, 0x00, 0x00, 0x20, 0x62, 0x61, 0x72, 0x0a, 0x50, 0x4b, 0x07, 0x08, + 0x91, 0xb0, 0x1b, 0xec, 0x19, 0x00, 0x00, 0x00, 0x19, 0x00, 0x00, 0x00, + 0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x08, 0x00, 0x00, 0x00, 0x6a, 0x85, + 0xf9, 0x54, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x62, 0x62, 0x61, 0x7a, 0x0a, 0x50, + 0x4b, 0x07, 0x08, 0xe1, 0x39, 0x7b, 0xcc, 0x04, 0x00, 0x00, 0x00, 0x04, + 0x00, 0x00, 0x00, 0x50, 0x4b, 0x01, 0x02, 0x14, 0x03, 0x14, 0x00, 0x08, + 0x00, 0x00, 0x00, 0x6a, 0x85, 0xf9, 0x54, 0x91, 0xb0, 0x1b, 0xec, 0x19, + 0x00, 0x00, 0x00, 0x19, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x61, 0x50, 0x4b, 0x01, 0x02, 0x14, 0x03, 0x14, 0x00, 0x08, 0x00, + 0x00, 0x00, 0x6a, 0x85, 0xf9, 0x54, 0xe1, 0x39, 0x7b, 0xcc, 0x04, 0x00, + 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x01, 0x48, 0x00, 0x00, 0x00, + 0x62, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x02, + 0x00, 0x5e, 0x00, 0x00, 0x00, 0x7b, 0x00, 0x00, 0x00, 0x00, 0x00, +}; + +// The fake descriptor is followed by a header signature, but its CRC is wrong. +// a content: "foo " + fake descriptor + "PK\x03\x04 bar\n" +const uint8_t kTestZipStoredDataDescriptorBadCrc[] = { + 0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x08, 0x00, 0x00, 0x00, 0x6a, 0x85, + 0xf9, 0x54, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x61, 0x66, 0x6f, 0x6f, 0x20, 0x50, + 0x4b, 0x07, 0x08, 0xef, 0xbe, 0xad, 0xde, 0x04, 0x00, 0x00, 0x00, 0x04, + 0x00, 0x00, 0x00, 0x50, 0x4b, 0x03, 0x04, 0x20, 0x62, 0x61, 0x72, 0x0a, + 0x50, 0x4b, 0x07, 0x08, 0x44, 0xdd, 0xe5, 0xaf, 0x1d, 0x00, 0x00, 0x00, + 0x1d, 0x00, 0x00, 0x00, 0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x08, 0x00, + 0x00, 0x00, 0x6a, 0x85, 0xf9, 0x54, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x62, 0x62, + 0x61, 0x7a, 0x0a, 0x50, 0x4b, 0x07, 0x08, 0xe1, 0x39, 0x7b, 0xcc, 0x04, + 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x50, 0x4b, 0x01, 0x02, 0x14, + 0x03, 0x14, 0x00, 0x08, 0x00, 0x00, 0x00, 0x6a, 0x85, 0xf9, 0x54, 0x44, + 0xdd, 0xe5, 0xaf, 0x1d, 0x00, 0x00, 0x00, 0x1d, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, + 0x01, 0x00, 0x00, 0x00, 0x00, 0x61, 0x50, 0x4b, 0x01, 0x02, 0x14, 0x03, + 0x14, 0x00, 0x08, 0x00, 0x00, 0x00, 0x6a, 0x85, 0xf9, 0x54, 0xe1, 0x39, + 0x7b, 0xcc, 0x04, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x01, + 0x4c, 0x00, 0x00, 0x00, 0x62, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, + 0x00, 0x02, 0x00, 0x02, 0x00, 0x5e, 0x00, 0x00, 0x00, 0x7f, 0x00, 0x00, + 0x00, 0x00, 0x00, +}; + // A single local file header whose 32-bit sizes are the 0xFFFFFFFF sentinel, // but whose Zip64 extra field (id 0x0001) declares a length of 4 bytes: too // short to carry the 8-byte size it promises. A conforming reader must reject @@ -225,6 +283,41 @@ ValidateTestZip(zr); } +// |a_suffix| is what follows the fake data descriptor in "a". +void ValidateTestZipStoredDataDescriptor(ZipReader& zr, + const std::string& a_suffix) { + ASSERT_EQ(zr.files().size(), 2u); + + std::vector<uint8_t> dec; + ASSERT_EQ(zr.files()[0].name(), "a"); + ASSERT_OK(zr.files()[0].Decompress(&dec)); + // "foo " + 16 bytes of fake descriptor + suffix. + std::string a = vec2str(dec); + ASSERT_EQ(a.size(), 4u + 16u + a_suffix.size()); + ASSERT_EQ(a.substr(0, 8), std::string("foo PK\x07\x08", 8)); + ASSERT_EQ(a.substr(20), a_suffix); + + ASSERT_EQ(zr.files()[1].name(), "b"); + ASSERT_OK(zr.files()[1].Decompress(&dec)); + ASSERT_EQ(vec2str(dec), "baz\n"); +} + +TEST(ZipReaderTest, ValidZipStoredDataDescriptor_OneShotParse) { + ZipReader zr; + ASSERT_OK(zr.Parse(TraceBlobView( + TraceBlob::CopyFrom(kTestZipStoredDataDescriptorBadNextSig, + sizeof(kTestZipStoredDataDescriptorBadNextSig))))); + ValidateTestZipStoredDataDescriptor(zr, " bar\n"); +} + +TEST(ZipReaderTest, ValidZipStoredDataDescriptor_OneByteChunks) { + ZipReader zr; + for (auto i : kTestZipStoredDataDescriptorBadNextSig) { + ASSERT_OK(zr.Parse(TraceBlobView(TraceBlob::CopyFrom(&i, 1)))); + } + ValidateTestZipStoredDataDescriptor(zr, " bar\n"); +} + TEST(ZipReaderTest, MalformedZip_InvalidSignature) { ZipReader zr; uint8_t content[sizeof(kTestZip)]; @@ -339,6 +432,14 @@ ASSERT_THAT(zr.files()[1].Decompress(&ignored), IsError()); } +TEST(ZipReaderTest, ValidZipStoredDataDescriptor_BadCrc) { + ZipReader zr; + ASSERT_OK(zr.Parse(TraceBlobView( + TraceBlob::CopyFrom(kTestZipStoredDataDescriptorBadCrc, + sizeof(kTestZipStoredDataDescriptorBadCrc))))); + ValidateTestZipStoredDataDescriptor(zr, "PK\x03\x04 bar\n"); +} + #endif // PERFETTO_BUILDFLAG(PERFETTO_ZLIB) } // namespace