Skip to content

Commit e71a46b

Browse files
mfklmarcin-krystianc
mfkl
and
marcin-krystianc
authored
Detect truncated GZip streams (#61768)
* System.IO.Compression tests: add StreamTruncation_IsDetected * System.IO.Compression: detect and throw on truncated streams * account for edge case * account for edge case in async version * rename nonZeroInput to nonEmptyInput * remove else to improve codegen * run StreamTruncation_IsDetected for all three types of compression streams * fixup test build and run * add stream corruption test * review feedback - cosmetics * make BrotliStream detect truncation * make StreamCorruption_IsDetected run for gzip only other types of stream can't detect corruption properly * skip byte corruption which results in correct decompression * code style * add zlib corruption test, no skipping needed * clarify why we skip bytes in gzip test * add and use truncated error data message Co-authored-by: Marcin Krystianc <marcin.krystianc@gmail.com>
1 parent c2a3191 commit e71a46b

File tree

11 files changed

+318
-13
lines changed

11 files changed

+318
-13
lines changed

‎src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,30 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Generic;
5+
using System.IO.Compression.Tests;
6+
using System.Linq;
57
using System.Text;
68
using System.Threading;
79
using System.Threading.Tasks;
810
using Xunit;
11+
using Xunit.Sdk;
912

1013
namespace System.IO.Compression
1114
{
1215
public abstract class CompressionStreamUnitTestBase : CompressionStreamTestBase
1316
{
1417
private const int TaskTimeout = 30 * 1000; // Generous timeout for official test runs
1518

19+
public enum TestScenario
20+
{
21+
ReadByte,
22+
ReadByteAsync,
23+
Read,
24+
ReadAsync,
25+
Copy,
26+
CopyAsync,
27+
}
28+
1629
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
1730
public virtual void FlushAsync_DuringWriteAsync()
1831
{
@@ -475,6 +488,85 @@ async Task<long> GetLengthAsync(CompressionLevel compressionLevel)
475488
Assert.True(fastestLength >= optimalLength);
476489
Assert.True(optimalLength >= smallestLength);
477490
}
491+
492+
[Theory]
493+
[InlineData(TestScenario.ReadAsync)]
494+
[InlineData(TestScenario.Read)]
495+
[InlineData(TestScenario.Copy)]
496+
[InlineData(TestScenario.CopyAsync)]
497+
[InlineData(TestScenario.ReadByte)]
498+
[InlineData(TestScenario.ReadByteAsync)]
499+
public async Task StreamTruncation_IsDetected(TestScenario scenario)
500+
{
501+
var buffer = new byte[16];
502+
byte[] source = Enumerable.Range(0, 64).Select(i => (byte)i).ToArray();
503+
byte[] compressedData;
504+
using (var compressed = new MemoryStream())
505+
using (Stream compressor = CreateStream(compressed, CompressionMode.Compress))
506+
{
507+
foreach (byte b in source)
508+
{
509+
compressor.WriteByte(b);
510+
}
511+
512+
compressor.Dispose();
513+
compressedData = compressed.ToArray();
514+
}
515+
516+
for (var i = 1; i <= compressedData.Length; i += 1)
517+
{
518+
bool expectException = i < compressedData.Length;
519+
using (var compressedStream = new MemoryStream(compressedData.Take(i).ToArray()))
520+
{
521+
using (Stream decompressor = CreateStream(compressedStream, CompressionMode.Decompress))
522+
{
523+
var decompressedStream = new MemoryStream();
524+
525+
try
526+
{
527+
switch (scenario)
528+
{
529+
case TestScenario.Copy:
530+
decompressor.CopyTo(decompressedStream);
531+
break;
532+
533+
case TestScenario.CopyAsync:
534+
await decompressor.CopyToAsync(decompressedStream);
535+
break;
536+
537+
case TestScenario.Read:
538+
while (ZipFileTestBase.ReadAllBytes(decompressor, buffer, 0, buffer.Length) != 0) { };
539+
break;
540+
541+
case TestScenario.ReadAsync:
542+
while (await ZipFileTestBase.ReadAllBytesAsync(decompressor, buffer, 0, buffer.Length) != 0) { };
543+
break;
544+
545+
case TestScenario.ReadByte:
546+
while (decompressor.ReadByte() != -1) { }
547+
break;
548+
549+
case TestScenario.ReadByteAsync:
550+
while (await decompressor.ReadByteAsync() != -1) { }
551+
break;
552+
}
553+
}
554+
catch (InvalidDataException e)
555+
{
556+
if (expectException)
557+
continue;
558+
559+
throw new XunitException($"An unexpected error occured while decompressing data:{e}");
560+
}
561+
562+
if (expectException)
563+
{
564+
throw new XunitException($"Truncated stream was decompressed successfully but exception was expected: length={i}/{compressedData.Length}");
565+
}
566+
}
567+
}
568+
}
569+
}
478570
}
479571

480572
internal sealed class BadWrappedStream : MemoryStream

‎src/libraries/Common/tests/System/IO/Compression/ZipTestHelper.cs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,17 @@ public static void ReadBytes(Stream stream, byte[] buffer, long bytesToRead)
6565
}
6666
}
6767

68+
public static async Task<int> ReadAllBytesAsync(Stream stream, byte[] buffer, int offset, int count)
69+
{
70+
int bytesRead;
71+
int totalRead = 0;
72+
while ((bytesRead = await stream.ReadAsync(buffer, offset + totalRead, count - totalRead)) != 0)
73+
{
74+
totalRead += bytesRead;
75+
}
76+
return totalRead;
77+
}
78+
6879
public static int ReadAllBytes(Stream stream, byte[] buffer, int offset, int count)
6980
{
7081
int bytesRead;
@@ -100,6 +111,11 @@ public static void StreamsEqual(Stream ast, Stream bst)
100111
StreamsEqual(ast, bst, -1);
101112
}
102113

114+
public static async Task StreamsEqualAsync(Stream ast, Stream bst)
115+
{
116+
await StreamsEqualAsync(ast, bst, -1);
117+
}
118+
103119
public static void StreamsEqual(Stream ast, Stream bst, int blocksToRead)
104120
{
105121
if (ast.CanSeek)
@@ -122,8 +138,44 @@ public static void StreamsEqual(Stream ast, Stream bst, int blocksToRead)
122138
if (blocksToRead != -1 && blocksRead >= blocksToRead)
123139
break;
124140

125-
ac = ReadAllBytes(ast, ad, 0, 4096);
126-
bc = ReadAllBytes(bst, bd, 0, 4096);
141+
ac = ReadAllBytes(ast, ad, 0, bufSize);
142+
bc = ReadAllBytes(bst, bd, 0, bufSize);
143+
144+
if (ac != bc)
145+
{
146+
bd = NormalizeLineEndings(bd);
147+
}
148+
149+
Assert.True(ArraysEqual<byte>(ad, bd, ac), "Stream contents not equal: " + ast.ToString() + ", " + bst.ToString());
150+
151+
blocksRead++;
152+
} while (ac == bufSize);
153+
}
154+
155+
public static async Task StreamsEqualAsync(Stream ast, Stream bst, int blocksToRead)
156+
{
157+
if (ast.CanSeek)
158+
ast.Seek(0, SeekOrigin.Begin);
159+
if (bst.CanSeek)
160+
bst.Seek(0, SeekOrigin.Begin);
161+
162+
const int bufSize = 4096;
163+
byte[] ad = new byte[bufSize];
164+
byte[] bd = new byte[bufSize];
165+
166+
int ac = 0;
167+
int bc = 0;
168+
169+
int blocksRead = 0;
170+
171+
//assume read doesn't do weird things
172+
do
173+
{
174+
if (blocksToRead != -1 && blocksRead >= blocksToRead)
175+
break;
176+
177+
ac = await ReadAllBytesAsync(ast, ad, 0, bufSize);
178+
bc = await ReadAllBytesAsync(bst, bd, 0, bufSize);
127179

128180
if (ac != bc)
129181
{
@@ -133,7 +185,7 @@ public static void StreamsEqual(Stream ast, Stream bst, int blocksToRead)
133185
Assert.True(ArraysEqual<byte>(ad, bd, ac), "Stream contents not equal: " + ast.ToString() + ", " + bst.ToString());
134186

135187
blocksRead++;
136-
} while (ac == 4096);
188+
} while (ac == bufSize);
137189
}
138190

139191
public static async Task IsZipSameAsDirAsync(string archiveFile, string directory, ZipArchiveMode mode)

‎src/libraries/System.IO.Compression.Brotli/src/Resources/Strings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,9 @@
175175
<data name="BrotliStream_Decompress_InvalidStream" xml:space="preserve">
176176
<value>BrotliStream.BaseStream returned more bytes than requested in Read.</value>
177177
</data>
178+
<data name="BrotliStream_Decompress_TruncatedData" xml:space="preserve">
179+
<value>Decoder ran into truncated data.</value>
180+
</data>
178181
<data name="IOCompressionBrotli_PlatformNotSupported" xml:space="preserve">
179182
<value>System.IO.Compression.Brotli is not supported on this platform.</value>
180183
</data>

‎src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ public sealed partial class BrotliStream : Stream
1515
private BrotliDecoder _decoder;
1616
private int _bufferOffset;
1717
private int _bufferCount;
18+
private bool _nonEmptyInput;
1819

1920
/// <summary>Reads a number of decompressed bytes into the specified byte array.</summary>
2021
/// <param name="buffer">The array used to store decompressed bytes.</param>
@@ -65,9 +66,12 @@ public override int Read(Span<byte> buffer)
6566
int bytesRead = _stream.Read(_buffer, _bufferCount, _buffer.Length - _bufferCount);
6667
if (bytesRead <= 0)
6768
{
69+
if (_nonEmptyInput && !buffer.IsEmpty)
70+
ThrowTruncatedInvalidData();
6871
break;
6972
}
7073

74+
_nonEmptyInput = true;
7175
_bufferCount += bytesRead;
7276

7377
if (_bufferCount > _buffer.Length)
@@ -150,9 +154,12 @@ async ValueTask<int> Core(Memory<byte> buffer, CancellationToken cancellationTok
150154
int bytesRead = await _stream.ReadAsync(_buffer.AsMemory(_bufferCount), cancellationToken).ConfigureAwait(false);
151155
if (bytesRead <= 0)
152156
{
157+
if (_nonEmptyInput && !buffer.IsEmpty)
158+
ThrowTruncatedInvalidData();
153159
break;
154160
}
155161

162+
_nonEmptyInput = true;
156163
_bufferCount += bytesRead;
157164

158165
if (_bufferCount > _buffer.Length)
@@ -231,5 +238,8 @@ private static void ThrowInvalidStream() =>
231238
// The stream is either malicious or poorly implemented and returned a number of
232239
// bytes larger than the buffer supplied to it.
233240
throw new InvalidDataException(SR.BrotliStream_Decompress_InvalidStream);
241+
242+
private static void ThrowTruncatedInvalidData() =>
243+
throw new InvalidDataException(SR.BrotliStream_Decompress_TruncatedData);
234244
}
235245
}

‎src/libraries/System.IO.Compression.Brotli/tests/System.IO.Compression.Brotli.Tests.csproj

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser</TargetFrameworks>
44
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -12,10 +12,16 @@
1212
Link="Common\System\IO\Compression\CompressionStreamTestBase.cs" />
1313
<Compile Include="$(CommonTestPath)System\IO\Compression\CompressionStreamUnitTestBase.cs"
1414
Link="Common\System\IO\Compression\CompressionStreamUnitTestBase.cs" />
15+
<Compile Include="$(CommonTestPath)System\IO\Compression\CRC.cs"
16+
Link="Common\System\IO\Compression\CRC.cs" />
17+
<Compile Include="$(CommonTestPath)System\IO\Compression\FileData.cs"
18+
Link="Common\System\IO\Compression\FileData.cs" />
1519
<Compile Include="$(CommonTestPath)System\IO\Compression\LocalMemoryStream.cs"
1620
Link="Common\System\IO\Compression\LocalMemoryStream.cs" />
1721
<Compile Include="$(CommonTestPath)System\IO\Compression\StreamHelpers.cs"
1822
Link="Common\System\IO\Compression\StreamHelpers.cs" />
23+
<Compile Include="$(CommonTestPath)System\IO\Compression\ZipTestHelper.cs"
24+
Link="Common\System\IO\Compression\ZipTestHelper.cs" />
1925
<Compile Include="$(CommonTestPath)System\IO\TempFile.cs"
2026
Link="Common\System\IO\TempFile.cs" />
2127
<Compile Include="$(CommonPath)System\Threading\Tasks\TaskToApm.cs"

‎src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Create.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,30 @@ public void CreateFromDirectory_IncludeBaseDirectory()
4545
}
4646
}
4747

48+
[Fact]
49+
public async Task CreateFromDirectory_IncludeBaseDirectoryAsync()
50+
{
51+
string folderName = zfolder("normal");
52+
string withBaseDir = GetTestFilePath();
53+
ZipFile.CreateFromDirectory(folderName, withBaseDir, CompressionLevel.Optimal, true);
54+
55+
IEnumerable<string> expected = Directory.EnumerateFiles(zfolder("normal"), "*", SearchOption.AllDirectories);
56+
using (ZipArchive actual_withbasedir = ZipFile.Open(withBaseDir, ZipArchiveMode.Read))
57+
{
58+
foreach (ZipArchiveEntry actualEntry in actual_withbasedir.Entries)
59+
{
60+
string expectedFile = expected.Single(i => Path.GetFileName(i).Equals(actualEntry.Name));
61+
Assert.StartsWith("normal", actualEntry.FullName);
62+
Assert.Equal(new FileInfo(expectedFile).Length, actualEntry.Length);
63+
using (Stream expectedStream = File.OpenRead(expectedFile))
64+
using (Stream actualStream = actualEntry.Open())
65+
{
66+
await StreamsEqualAsync(expectedStream, actualStream);
67+
}
68+
}
69+
}
70+
}
71+
4872
[Fact]
4973
public void CreateFromDirectoryUnicode()
5074
{

‎src/libraries/System.IO.Compression/src/Resources/Strings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,9 @@
278278
<data name="SplitSpanned" xml:space="preserve">
279279
<value>Split or spanned archives are not supported.</value>
280280
</data>
281+
<data name="TruncatedData" xml:space="preserve">
282+
<value>Found truncated data while decoding.</value>
283+
</data>
281284
<data name="UnexpectedEndOfStream" xml:space="preserve">
282285
<value>Zip file corrupt: unexpected end of stream reached.</value>
283286
</data>

‎src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,15 @@ internal int ReadCore(Span<byte> buffer)
279279
int n = _stream.Read(_buffer, 0, _buffer.Length);
280280
if (n <= 0)
281281
{
282+
// - Inflater didn't return any data although a non-empty output buffer was passed by the caller.
283+
// - More input is needed but there is no more input available.
284+
// - Inflation is not finished yet.
285+
// - Provided input wasn't completely empty
286+
// In such case, we are dealing with a truncated input stream.
287+
if (!buffer.IsEmpty && !_inflater.Finished() && _inflater.NonEmptyInput())
288+
{
289+
ThrowTruncatedInvalidData();
290+
}
282291
break;
283292
}
284293
else if (n > _buffer.Length)
@@ -347,6 +356,9 @@ private static void ThrowGenericInvalidData() =>
347356
// bytes < 0 || > than the buffer supplied to it.
348357
throw new InvalidDataException(SR.GenericInvalidData);
349358

359+
private static void ThrowTruncatedInvalidData() =>
360+
throw new InvalidDataException(SR.TruncatedData);
361+
350362
public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? asyncCallback, object? asyncState) =>
351363
TaskToApm.Begin(ReadAsync(buffer, offset, count, CancellationToken.None), asyncCallback, asyncState);
352364

@@ -416,6 +428,15 @@ async ValueTask<int> Core(Memory<byte> buffer, CancellationToken cancellationTok
416428
int n = await _stream.ReadAsync(new Memory<byte>(_buffer, 0, _buffer.Length), cancellationToken).ConfigureAwait(false);
417429
if (n <= 0)
418430
{
431+
// - Inflater didn't return any data although a non-empty output buffer was passed by the caller.
432+
// - More input is needed but there is no more input available.
433+
// - Inflation is not finished yet.
434+
// - Provided input wasn't completely empty
435+
// In such case, we are dealing with a truncated input stream.
436+
if (!_inflater.Finished() && _inflater.NonEmptyInput() && !buffer.IsEmpty)
437+
{
438+
ThrowTruncatedInvalidData();
439+
}
419440
break;
420441
}
421442
else if (n > _buffer.Length)
@@ -436,6 +457,7 @@ async ValueTask<int> Core(Memory<byte> buffer, CancellationToken cancellationTok
436457
// decompress into at least one byte of output, but it's a reasonable approximation for the 99% case. If it's
437458
// wrong, it just means that a caller using zero-byte reads as a way to delay getting a buffer to use for a
438459
// subsequent call may end up getting one earlier than otherwise preferred.
460+
Debug.Assert(bytesRead == 0);
439461
break;
440462
}
441463
}
@@ -893,6 +915,10 @@ public async Task CopyFromSourceToDestinationAsync()
893915

894916
// Now, use the source stream's CopyToAsync to push directly to our inflater via this helper stream
895917
await _deflateStream._stream.CopyToAsync(this, _arrayPoolBuffer.Length, _cancellationToken).ConfigureAwait(false);
918+
if (!_deflateStream._inflater.Finished())
919+
{
920+
ThrowTruncatedInvalidData();
921+
}
896922
}
897923
finally
898924
{
@@ -925,6 +951,10 @@ public void CopyFromSourceToDestination()
925951

926952
// Now, use the source stream's CopyToAsync to push directly to our inflater via this helper stream
927953
_deflateStream._stream.CopyTo(this, _arrayPoolBuffer.Length);
954+
if (!_deflateStream._inflater.Finished())
955+
{
956+
ThrowTruncatedInvalidData();
957+
}
928958
}
929959
finally
930960
{

0 commit comments

Comments
 (0)