From 2ade216989f5226cce562945bb03e6a351f2d8b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 04:44:45 +0000 Subject: [PATCH] Check a later Gzip member's CRC before accepting a short trailer [patch] To allow concatenated members, the Gzip end check accepted any input whose last four bytes read as a size below the output so far. In truncated input those bytes are payload, and stored blocks copy payload verbatim, so a cut just after a zero or small little-endian field "succeeded" with partial output on the span, stream and async paths. Accept a shorter trailer only when its CRC-32 matches the output it claims to cover: on the span path by checking that suffix directly, and on the stream paths by deriving the CRC of the output since each read began, because GZipStream returns from a read at every member's end. Fixes ktsu-dev/Essentials#171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW671w9qNTjehHKvwnt --- CLAUDE.md | 2 +- Essentials.Tests/TruncatedCompressionTests.cs | 113 ++++++++++++++ Shared/CompressedStreamEnd.cs | 145 +++++++++++++++++- 3 files changed, 251 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fbc3d4c..d17e8e2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,7 +46,7 @@ This is a .NET library (`ktsu.Essentials`) providing high-performance interfaces - `Essentials/IDiscreteDistribution.cs` - `IDistribution` plus `Pmf`/`LogPmf`, and a default `Quantile` that bisects the CDF. That default is an explicit reimplementation of `IDistribution.Quantile`, which CA1033 misreads as an explicit implementation on a class; the suppression on it says why - `Shared/DeflateBound.cs` - zlib's `deflateBound` plus container overhead, linked into the Deflate, Gzip and ZLib providers as their `GetMaxCompressedLength` - `Shared/BoundedDecompression.cs` - The span overload of `TryDecompress` for Deflate, Gzip and ZLib: reads the decompressing stream straight into the destination and fails once it is full and more output remains, so the destination bounds the work. See issue #122. Brotli uses `BrotliDecoder` directly instead, which reports a truncated stream itself -- `Shared/CompressedStreamEnd.cs` - Gzip's CRC-32 and length trailer and ZLib's Adler-32, checked against the output on every decompression path, because `GZipStream` and `ZLibStream` treat input that runs out as a clean end of stream. Raw Deflate has no trailer, so its truncation cannot be detected. See issue #80 +- `Shared/CompressedStreamEnd.cs` - Gzip's CRC-32 and length trailer and ZLib's Adler-32, checked against the output on every decompression path, because `GZipStream` and `ZLibStream` treat input that runs out as a clean end of stream. A Gzip trailer shorter than the whole output is a later member's, and is accepted only when its CRC-32 matches that much output from the end (issue #171). Raw Deflate has no trailer, so its truncation cannot be detected. See issue #80 - `Shared/StreamChunks.cs` - Reads a stream a chunk at a time through a pooled buffer, linked into the FNV hash and the Xor, Caesar and BitRotate obfuscation providers, whose stream paths otherwise made a call per byte. See issue #91 - `Shared/SpecialFunctions.cs` - Incomplete gamma, incomplete beta, error function, normal quantile and cancellation-free `ExpM1`/`Log1P`, linked into the Normal, LogNormal, Exponential, Binomial, Poisson and Geometric providers rather than placed in the interfaces-only package - `Shared/DistributionArguments.cs` - Parameter validation shared by every distribution provider, linked in the same way diff --git a/Essentials.Tests/TruncatedCompressionTests.cs b/Essentials.Tests/TruncatedCompressionTests.cs index a65a1a5..077ffae 100644 --- a/Essentials.Tests/TruncatedCompressionTests.cs +++ b/Essentials.Tests/TruncatedCompressionTests.cs @@ -107,6 +107,119 @@ public void Gzip_Accepts_Concatenated_Members() CollectionAssert.AreEqual((byte[])[.. first, .. second], gzip.Decompress(concatenated)); } + [TestMethod] + public async Task Gzip_Rejects_A_Stored_Block_Payload_Cut_After_A_Zeroed_Field() + { + // Stored blocks copy the payload verbatim, so a cut just after four zero bytes leaves a "trailer" whose + // length field reads as 0 and whose CRC field is payload data (issue #171). + ICompressionProvider gzip = new GzipCompressionProvider(); + byte[] payload = new byte[100_000]; + new Random(42).NextBytes(payload); + payload.AsSpan(50_000, 4).Clear(); + byte[] compressed = gzip.Compress(payload); + int zeros = 20 + compressed.AsSpan(20).IndexOf(new byte[4]); + byte[] truncated = compressed[..(zeros + 4)]; + + await AssertRejectedOnEveryPath(gzip, truncated, payload.Length, $"cut at {truncated.Length}").ConfigureAwait(false); + } + + [TestMethod] + public async Task Gzip_Rejects_A_Stored_Block_Payload_Cut_At_Every_Offset() + { + // Random bytes with zero runs and small little-endian integers in them, which deflate stores verbatim: + // whatever the cut, the input's last eight bytes are payload, not a trailer. + ICompressionProvider gzip = new GzipCompressionProvider(); + byte[] payload = StoredBlockPayload(20_000); + byte[] compressed = gzip.Compress(payload); + + for (int length = 1; length < compressed.Length; length++) + { + await AssertRejectedOnEveryPath(gzip, compressed[..length], payload.Length, $"cut at {length} of {compressed.Length}").ConfigureAwait(false); + } + } + + [TestMethod] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1849:Call async methods when in an async method", Justification = "Deliberately exercises the synchronous stream path alongside the asynchronous one.")] + public async Task Gzip_Accepts_Concatenated_Members_On_Every_Path() + { + ICompressionProvider gzip = new GzipCompressionProvider(); + byte[] first = StoredBlockPayload(150_000); + byte[] second = [.. Payload().AsSpan(0, 5000)]; + byte[] third = StoredBlockPayload(3000); + byte[] expected = [.. first, .. second, .. third]; + byte[] concatenated = [.. gzip.Compress(first), .. gzip.Compress(second), .. gzip.Compress(third)]; + + byte[] destination = new byte[expected.Length]; + Assert.IsTrue(gzip.TryDecompress(concatenated, destination, out int written), "span rejected concatenated members"); + Assert.AreEqual(expected.Length, written); + CollectionAssert.AreEqual(expected, destination); + + using (MemoryStream source = new(concatenated)) + using (MemoryStream sink = new()) + { + Assert.IsTrue(gzip.TryDecompress(source, sink), "stream rejected concatenated members"); + CollectionAssert.AreEqual(expected, sink.ToArray()); + } + + using (AsyncOnlyStream source = new(concatenated)) + using (AsyncOnlyStream sink = new()) + { + Assert.IsTrue(await gzip.TryDecompressAsync(source, sink, TestContext.CancellationToken).ConfigureAwait(false), "async rejected concatenated members"); + CollectionAssert.AreEqual(expected, sink.ToArray()); + } + } + + [TestMethod] + public async Task Gzip_Rejects_Concatenated_Members_Whose_Last_Member_Is_Cut() + { + ICompressionProvider gzip = new GzipCompressionProvider(); + byte[] first = gzip.Compress(Payload()); + byte[] last = gzip.Compress(StoredBlockPayload(4000)); + byte[] concatenated = [.. first, .. last]; + + for (int length = first.Length + 1; length < concatenated.Length; length++) + { + await AssertRejectedOnEveryPath(gzip, concatenated[..length], concatenated.Length * 2, $"cut at {length} of {concatenated.Length}").ConfigureAwait(false); + } + } + + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1849:Call async methods when in an async method", Justification = "Deliberately exercises the synchronous stream path alongside the asynchronous one.")] + private async Task AssertRejectedOnEveryPath(ICompressionProvider provider, byte[] truncated, int capacity, string what) + { + Assert.IsFalse(provider.TryDecompress(truncated, new byte[capacity], out int written), $"span accepted {what}"); + Assert.AreEqual(0, written, $"span reported output for {what}"); + + using (MemoryStream source = new(truncated)) + using (MemoryStream sink = new()) + { + Assert.IsFalse(provider.TryDecompress(source, sink), $"stream accepted {what}"); + } + + using (AsyncOnlyStream source = new(truncated)) + using (AsyncOnlyStream sink = new()) + { + Assert.IsFalse(await provider.TryDecompressAsync(source, sink, TestContext.CancellationToken).ConfigureAwait(false), $"async accepted {what}"); + } + + Assert.ThrowsExactly(() => provider.Decompress(truncated), $"Decompress returned short output for {what}"); + } + + /// Random bytes, which deflate leaves in stored blocks, with zero runs and small integers through them. + private static byte[] StoredBlockPayload(int length) + { + Random random = new(171); + byte[] payload = new byte[length]; + random.NextBytes(payload); + for (int i = 0; i + 8 <= length; i += 300 + random.Next(300)) + { + // A zeroed 32-bit field after a small little-endian one, as binary formats are full of + payload.AsSpan(i, 8).Clear(); + payload[i] = (byte)random.Next(1, 16); + } + + return payload; + } + private static IEnumerable Cuts(int length) { foreach (int cut in new[] { 1, 2, 3, 4, 8, 12, length / 3, length / 2, length - 9, length - 8, length - 5, length - 4, length - 2, length - 1 }) diff --git a/Shared/CompressedStreamEnd.cs b/Shared/CompressedStreamEnd.cs index 13f1b3a..f522b55 100644 --- a/Shared/CompressedStreamEnd.cs +++ b/Shared/CompressedStreamEnd.cs @@ -3,6 +3,7 @@ namespace ktsu.Essentials; using System; +using System.Collections.Generic; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -22,9 +23,12 @@ namespace ktsu.Essentials; /// /// The input's last bytes are its trailer only when the compressed data is all the input there is, so data /// followed by anything else is reported as a failure. A Gzip file is allowed several members, read back to back -/// as gzip itself reads them; its trailer then describes only the last member, so a trailer whose size is less -/// than the whole output is taken for one, the members before it having been checked by the decompressor as it -/// crossed their trailers. +/// as gzip itself reads them; its trailer then describes only the last member, the members before it having +/// been checked by the decompressor as it crossed their trailers. A trailer whose size is less than the whole +/// output is accepted only when its CRC-32 matches the output that size from the end, and on the stream paths +/// only when that output began at the start of a read, since the decompressor returns from a read at the end of +/// each member. Taking any shorter size on trust accepted cut-off input whose last bytes happened to read as a +/// small number, which binary payloads that deflate stores verbatim make common (issue #171). /// /// internal abstract class CompressedStreamEnd @@ -43,6 +47,12 @@ internal abstract class CompressedStreamEnd /// if the stream reached its own end. internal abstract bool EndsWith(ReadOnlySpan inputTail); + /// Whether the input ended in the trailer calls for, with all of the output at hand. + /// The last bytes of the input, up to of them. + /// Everything the decompressor produced, already passed to . + /// if the stream reached its own end. + internal virtual bool EndsWith(ReadOnlySpan inputTail, ReadOnlySpan output) => EndsWith(inputTail); + /// /// Whether is everything describes, rather than what it /// held before it was cut off. @@ -59,7 +69,7 @@ internal bool IsComplete(ReadOnlySpan output, ReadOnlySpan input) } Append(output); - return EndsWith(input[Math.Max(0, input.Length - MaxTrailerLength)..]); + return EndsWith(input[Math.Max(0, input.Length - MaxTrailerLength)..], output); } /// @@ -115,25 +125,84 @@ internal async Task CopyToAsync(Stream compressedData, Stream destination, /// Gzip's trailer: the CRC-32 of the member's output, then its length modulo 2^32, both little-endian. internal sealed class Gzip : CompressedStreamEnd { + // Where each run passed to Append began, with the CRC-32 of the output before it. GZipStream returns + // from a read at the end of each member, so a later member's output starts at one of these. + private readonly List<(long Offset, uint Crc)> runStarts = []; + private uint crc = 0xFFFFFFFFu; private uint length; + private long total; internal override void Append(ReadOnlySpan output) { + if (output.IsEmpty) + { + return; + } + + runStarts.Add((total, ~crc)); crc = Crc32.Update(crc, output); length = unchecked(length + (uint)output.Length); + total += output.Length; } internal override bool EndsWith(ReadOnlySpan inputTail) + { + if (!TryReadTrailer(inputTail, out uint trailerCrc, out uint trailerLength)) + { + return false; + } + + if (trailerCrc == ~crc && trailerLength == length) + { + return true; + } + + // A later member: its trailer covers the output since one of the runs began. CRC-32 is linear, so the + // CRC of that suffix follows from the CRC of everything and the CRC of what came before it. + for (int i = runStarts.Count - 1; i > 0; i--) + { + (long offset, uint crcBefore) = runStarts[i]; + long suffixLength = total - offset; + if (unchecked((uint)suffixLength) == trailerLength && (~crc ^ Crc32.Shift(crcBefore, suffixLength)) == trailerCrc) + { + return true; + } + } + + return false; + } + + internal override bool EndsWith(ReadOnlySpan inputTail, ReadOnlySpan output) + { + if (!TryReadTrailer(inputTail, out uint trailerCrc, out uint trailerLength)) + { + return false; + } + + if (trailerCrc == ~crc && trailerLength == length) + { + return true; + } + + // A later member: with the whole output here, check its trailer against the output it claims to cover. + return trailerLength > 0 + && trailerLength < (uint)output.Length + && ~Crc32.Update(0xFFFFFFFFu, output[^(int)trailerLength..]) == trailerCrc; + } + + private static bool TryReadTrailer(ReadOnlySpan inputTail, out uint trailerCrc, out uint trailerLength) { if (inputTail.Length < 8) { + trailerCrc = 0; + trailerLength = 0; return false; } - uint trailerCrc = ReadUInt32LittleEndian(inputTail[^8..]); - uint trailerLength = ReadUInt32LittleEndian(inputTail[^4..]); - return (trailerCrc == ~crc && trailerLength == length) || trailerLength < length; + trailerCrc = ReadUInt32LittleEndian(inputTail[^8..]); + trailerLength = ReadUInt32LittleEndian(inputTail[^4..]); + return true; } } @@ -184,8 +253,19 @@ private static uint ReadUInt32LittleEndian(ReadOnlySpan bytes) /// The CRC-32 Gzip uses (the reflected IEEE 802.3 polynomial), table driven. private static class Crc32 { + private const uint Polynomial = 0xEDB88320u; + private static readonly uint[] Table = CreateTable(); + // x^(2^k) modulo the polynomial, for k = 0 to 31, as zlib's crc32_combine uses. + private static readonly uint[] PowersOfX = CreatePowersOfX(); + + /// + /// The CRC-32 of 's data followed by zero bytes, less + /// the CRC-32 of those zeros alone: XOR with the CRC-32 of any data that long gives the CRC-32 of both. + /// + internal static uint Shift(uint crc, long byteCount) => Multiply(XToThe(byteCount, 3), crc); + internal static uint Update(uint crc, ReadOnlySpan data) { foreach (byte value in data) @@ -204,7 +284,7 @@ private static uint[] CreateTable() uint c = n; for (int k = 0; k < 8; k++) { - c = (c & 1) != 0 ? 0xEDB88320u ^ (c >> 1) : c >> 1; + c = (c & 1) != 0 ? Polynomial ^ (c >> 1) : c >> 1; } table[n] = c; @@ -212,6 +292,55 @@ private static uint[] CreateTable() return table; } + + /// Multiplies two polynomials modulo the CRC-32 polynomial, in its reflected bit order. + private static uint Multiply(uint a, uint b) + { + uint product = 0; + for (uint mask = 1u << 31; mask != 0 && a != 0; mask >>= 1) + { + if ((a & mask) != 0) + { + product ^= b; + a ^= mask; + } + + b = (b & 1) != 0 ? (b >> 1) ^ Polynomial : b >> 1; + } + + return product; + } + + /// x^(n * 2^k) modulo the CRC-32 polynomial. + private static uint XToThe(long n, int k) + { + uint power = 1u << 31; + while (n != 0) + { + if ((n & 1) != 0) + { + power = Multiply(PowersOfX[k & 31], power); + } + + n >>= 1; + k++; + } + + return power; + } + + private static uint[] CreatePowersOfX() + { + uint[] powers = new uint[32]; + uint power = 1u << 30; + powers[0] = power; + for (int k = 1; k < powers.Length; k++) + { + powers[k] = power = Multiply(power, power); + } + + return powers; + } } ///