From 59c7160331f5779f98a65a3c0142d46151d88587 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Thu, 15 Oct 2026 14:16:39 +0100 Subject: [PATCH] Pool ZIP directory search buffers and bound signature scans --- .../Zip/SeekableZipHeaderFactory.Async.cs | 6 +- .../Common/Zip/SeekableZipHeaderFactory.cs | 42 +++--- .../Zip/ZipDirectorySearchTests.cs | 130 ++++++++++++++++++ 3 files changed, 156 insertions(+), 22 deletions(-) create mode 100644 tests/SharpCompress.Test/Zip/ZipDirectorySearchTests.cs diff --git a/src/SharpCompress/Common/Zip/SeekableZipHeaderFactory.Async.cs b/src/SharpCompress/Common/Zip/SeekableZipHeaderFactory.Async.cs index e9065f80..f05affe1 100644 --- a/src/SharpCompress/Common/Zip/SeekableZipHeaderFactory.Async.cs +++ b/src/SharpCompress/Common/Zip/SeekableZipHeaderFactory.Async.cs @@ -106,10 +106,8 @@ internal sealed partial class SeekableZipHeaderFactory var span = memory.Span; span.Reverse(); - // don't exclude the minimum eocd region, otherwise you fail to locate the header in empty zip files - var max_search_area = len; // - MINIMUM_EOCD_LENGTH; - - for (var pos_from_end = 0; pos_from_end < max_search_area; ++pos_from_end) + // Include the minimum EOCD region, but never read beyond the populated span. + for (var pos_from_end = 0; pos_from_end <= len - needle.Length; ++pos_from_end) { if (IsMatch(span, pos_from_end, needle)) { diff --git a/src/SharpCompress/Common/Zip/SeekableZipHeaderFactory.cs b/src/SharpCompress/Common/Zip/SeekableZipHeaderFactory.cs index c20163b9..875cbc90 100644 --- a/src/SharpCompress/Common/Zip/SeekableZipHeaderFactory.cs +++ b/src/SharpCompress/Common/Zip/SeekableZipHeaderFactory.cs @@ -26,7 +26,7 @@ internal sealed partial class SeekableZipHeaderFactory : ZipHeaderFactory { var reader = new BinaryReader(stream); - SeekBackToHeader(stream, reader); + SeekBackToHeader(stream); var eocd_location = stream.Position; var entry = new DirectoryEndHeader(); @@ -102,7 +102,7 @@ internal sealed partial class SeekableZipHeaderFactory : ZipHeaderFactory return true; } - private static void SeekBackToHeader(Stream stream, BinaryReader reader) + private static void SeekBackToHeader(Stream stream) { // Minimum EOCD length if (stream.Length < MINIMUM_EOCD_LENGTH) @@ -118,24 +118,30 @@ internal sealed partial class SeekableZipHeaderFactory : ZipHeaderFactory : MAX_SEARCH_LENGTH_FOR_EOCD; stream.Seek(-len, SeekOrigin.End); - var seek = reader.ReadBytes(len); - - // Search in reverse - Array.Reverse(seek); - - // don't exclude the minimum eocd region, otherwise you fail to locate the header in empty zip files - var max_search_area = len; // - MINIMUM_EOCD_LENGTH; - - for (var pos_from_end = 0; pos_from_end < max_search_area; ++pos_from_end) + var seek = ArrayPool.Shared.Rent(len); + try { - if (IsMatch(seek, pos_from_end, needle)) - { - stream.Seek(-pos_from_end, SeekOrigin.End); - return; - } - } + stream.ReadExact(seek, 0, len); + // Only reverse and search the bytes read, not the unused tail of a pooled array. + var span = seek.AsSpan(0, len); + span.Reverse(); - throw new ArchiveException("Failed to locate the Zip Header"); + // Include the minimum EOCD region so empty ZIP archives can be found. + for (var pos_from_end = 0; pos_from_end <= len - needle.Length; ++pos_from_end) + { + if (IsMatch(span, pos_from_end, needle)) + { + stream.Seek(-pos_from_end, SeekOrigin.End); + return; + } + } + + throw new ArchiveException("Failed to locate the Zip Header"); + } + finally + { + ArrayPool.Shared.Return(seek); + } } internal LocalEntryHeader GetLocalHeader( diff --git a/tests/SharpCompress.Test/Zip/ZipDirectorySearchTests.cs b/tests/SharpCompress.Test/Zip/ZipDirectorySearchTests.cs new file mode 100644 index 00000000..3ecd937f --- /dev/null +++ b/tests/SharpCompress.Test/Zip/ZipDirectorySearchTests.cs @@ -0,0 +1,130 @@ +using System; +using System.Buffers.Binary; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives.Zip; +using SharpCompress.Common; +using SharpCompress.Common.Zip; +using SharpCompress.Readers; +using SharpCompress.Test.Mocks; +using Xunit; + +namespace SharpCompress.Test.Zip; + +public class ZipDirectorySearchTests +{ + [Theory] + [InlineData(0, false)] + [InlineData(0, true)] + [InlineData(1, false)] + [InlineData(1, true)] + [InlineData(4096, false)] + [InlineData(4096, true)] + [InlineData(ushort.MaxValue, false)] + [InlineData(ushort.MaxValue, true)] + public async Task Empty_Archive_With_Comment_And_Short_Reads(int commentLength, bool async) + { + var bytes = new byte[22 + commentLength]; + BinaryPrimitives.WriteUInt32LittleEndian(bytes, 0x06054b50); + BinaryPrimitives.WriteUInt16LittleEndian(bytes.AsSpan(20), (ushort)commentLength); + bytes.AsSpan(22).Fill((byte)'a'); + using var input = new ShortReadStream(new MemoryStream(bytes), maxReadSize: 7); + if (async) + { + await using var archive = await ZipArchive.OpenAsyncArchive( + input, + ReaderOptions.ForExternalStream + ); + Assert.Empty(await archive.EntriesAsync.ToListAsync()); + } + else + { + using var archive = ZipArchive.OpenArchive(input, ReaderOptions.ForExternalStream); + Assert.Empty(archive.Entries); + } + Assert.True(input.CanRead); + } + + [Theory] + [InlineData(1, false)] + [InlineData(1, true)] + [InlineData(2, false)] + [InlineData(2, true)] + [InlineData(3, false)] + [InlineData(3, true)] + public async Task Missing_Directory_With_Partial_Signature_Throws_ArchiveException( + int prefixLength, + bool async + ) + { + var bytes = new byte[31]; + var signature = new byte[] { 0x06, 0x05, 0x4b, 0x50 }; + for (var i = 0; i < prefixLength; i++) + { + bytes[prefixLength - i - 1] = signature[i]; + } + using var input = new MemoryStream(bytes); + var factory = new SeekableZipHeaderFactory(null, new ArchiveEncoding()); + if (async) + { + await Assert.ThrowsAsync(async () => + await factory.ReadSeekableHeaderAsync(input).ToListAsync() + ); + } + else + { + Assert.Throws(() => factory.ReadSeekableHeader(input).ToList()); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Directory_Search_Rejects_Premature_End_Of_Stream(bool async) + { + using var input = new PrematureEndStream(); + var factory = new SeekableZipHeaderFactory(null, new ArchiveEncoding()); + if (async) + { + await Assert.ThrowsAsync(async () => + await factory.ReadSeekableHeaderAsync(input).ToListAsync() + ); + } + else + { + Assert.Throws(() => + factory.ReadSeekableHeader(input).ToList() + ); + } + } + + private sealed class PrematureEndStream : MemoryStream + { + public PrematureEndStream() + : base(new byte[31]) { } + + public override int Read(byte[] buffer, int offset, int count) => + base.Read(buffer, offset, (int)Math.Min(count, Math.Max(0, 10 - Position))); + + public override Task ReadAsync( + byte[] buffer, + int offset, + int count, + CancellationToken cancellationToken + ) => Task.FromResult(Read(buffer, offset, count)); + +#if !LEGACY_DOTNET + public override ValueTask ReadAsync( + Memory buffer, + CancellationToken cancellationToken = default + ) => + new( + base.Read( + buffer.Span.Slice(0, (int)Math.Min(buffer.Length, Math.Max(0, 10 - Position))) + ) + ); +#endif + } +}