Check a later Gzip member's CRC before accepting a short trailer [patch]

CLAUDE.md

@@ -46,7 +46,7 @@ This is a .NET library (`ktsu.Essentials`) providing high-performance interfaces
 - `Essentials/IDiscreteDistribution.cs` - `IDistribution<int>` plus `Pmf`/`LogPmf`, and a default `Quantile` that bisects the CDF. That default is an explicit reimplementation of `IDistribution<int>.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

Essentials.Tests/TruncatedCompressionTests.cs

@@ -107,6 +107,119 @@
 		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<InvalidOperationException>(() => provider.Decompress(truncated), $"Decompress returned short output for {what}");
+	}
+
+	/// <summary>Random bytes, which deflate leaves in stored blocks, with zero runs and small integers through them.</summary>
+	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<int> 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 })

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;
 /// <para>
 /// 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).
 /// </para>
 /// </remarks>
 internal abstract class CompressedStreamEnd
@@ -43,6 +47,12 @@ internal abstract class CompressedStreamEnd
 	/// <returns><see langword="true"/> if the stream reached its own end.</returns>
 	internal abstract bool EndsWith(ReadOnlySpan<byte> inputTail);
 
+	/// <summary>Whether the input ended in the trailer <paramref name="output"/> calls for, with all of the output at hand.</summary>
+	/// <param name="inputTail">The last bytes of the input, up to <see cref="MaxTrailerLength"/> of them.</param>
+	/// <param name="output">Everything the decompressor produced, already passed to <see cref="Append"/>.</param>
+	/// <returns><see langword="true"/> if the stream reached its own end.</returns>
+	internal virtual bool EndsWith(ReadOnlySpan<byte> inputTail, ReadOnlySpan<byte> output) => EndsWith(inputTail);
+
 	/// <summary>
 	/// Whether <paramref name="output"/> is everything <paramref name="input"/> describes, rather than what it
 	/// held before it was cut off.
@@ -59,7 +69,7 @@ internal bool IsComplete(ReadOnlySpan<byte> output, ReadOnlySpan<byte> input)
 		}
 
 		Append(output);
-		return EndsWith(input[Math.Max(0, input.Length - MaxTrailerLength)..]);
+		return EndsWith(input[Math.Max(0, input.Length - MaxTrailerLength)..], output);
 	}
 
 	/// <summary>
@@ -115,25 +125,84 @@ internal async Task<bool> CopyToAsync(Stream compressedData, Stream destination,
 	/// <summary>Gzip's trailer: the CRC-32 of the member's output, then its length modulo 2^32, both little-endian.</summary>
 	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<byte> 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<byte> 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<byte> inputTail, ReadOnlySpan<byte> 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<byte> 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<byte> bytes)
 	/// <summary>The CRC-32 Gzip uses (the reflected IEEE 802.3 polynomial), table driven.</summary>
 	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();
+
+		/// <summary>
+		/// The CRC-32 of <paramref name="crc"/>'s data followed by <paramref name="byteCount"/> 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.
+		/// </summary>
+		internal static uint Shift(uint crc, long byteCount) => Multiply(XToThe(byteCount, 3), crc);
+
 		internal static uint Update(uint crc, ReadOnlySpan<byte> data)
 		{
 			foreach (byte value in data)
@@ -204,14 +284,63 @@ 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;
 			}
 
 			return table;
 		}
+
+		/// <summary>Multiplies two polynomials modulo the CRC-32 polynomial, in its reflected bit order.</summary>
+		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;
+		}
+
+		/// <summary>x^(n * 2^k) modulo the CRC-32 polynomial.</summary>
+		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;
+		}
 	}
 
 	/// <summary>