Fix release review findings in RAR, GZip, and split ZIP parsing

This commit is contained in:
Adam Hathcock
2026-10-10 08:22:55 +01:00
parent e4193af83b
commit ab3ac18e98
16 changed files with 648 additions and 57 deletions

View File

@@ -1,10 +1,14 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using AwesomeAssertions;
using SharpCompress.Archives.GZip;
using SharpCompress.Compressors.Deflate;
using SharpCompress.Readers;
using SharpCompress.Readers.GZip;
using SharpCompress.Test.Mocks;
using Xunit;
@@ -17,6 +21,192 @@ public class GZipExtraFieldTests
{
private static readonly byte[] EXPECTED_PAYLOAD = [.. "gzip payload"u8];
public static IEnumerable<object[]> FragmentedExtraFields()
{
foreach (var api in new[] { "Stream", "Reader", "Archive" })
{
foreach (var useAsync in new[] { false, true })
{
foreach (var seekable in new[] { false, true })
{
foreach (var extraLength in new[] { 0, 32768, 65535 })
{
foreach (var chunkSize in new[] { 1, 7, 1024 })
{
if (api != "Archive" || seekable)
{
yield return [api, useAsync, seekable, extraLength, chunkSize];
}
}
}
}
}
}
}
[Theory]
[MemberData(nameof(FragmentedExtraFields))]
public async Task GZip_Read_WithFragmentedExtraField_ShouldReturnPayload(
string api,
bool useAsync,
bool seekable,
int extraLength,
int chunkSize
)
{
using var input = new ShortReadStream(
new MemoryStream(CreateGZipWithExtraField(extraLength, "payload.bin")),
chunkSize,
seekable
);
using var asyncOnly = new AsyncOnlyStream(input, disposeStream: false);
Assert.Equal(
EXPECTED_PAYLOAD,
await ReadPayload(useAsync ? asyncOnly : input, api, useAsync, CancellationToken.None)
);
Assert.False(input.IsDisposed);
}
public static IEnumerable<object[]> TruncatedExtraFields()
{
foreach (var api in new[] { "Stream", "Reader", "Archive" })
{
foreach (var useAsync in new[] { false, true })
{
foreach (var length in new[] { 1, 9, 10, 11, 12, 13, 43 })
{
yield return [api, useAsync, length];
}
}
}
}
[Theory]
[MemberData(nameof(TruncatedExtraFields))]
public async Task GZip_Read_RejectsTruncatedExtraField(string api, bool useAsync, int length)
{
using var input = new ShortReadStream(
new MemoryStream(CreateGZipWithExtraField(32).Take(length).ToArray())
);
await Assert.ThrowsAsync<ZlibException>(() =>
ReadPayload(input, api, useAsync, CancellationToken.None)
);
Assert.False(input.IsDisposed);
}
[Theory]
[InlineData(false, 1)]
[InlineData(true, 1)]
[InlineData(false, 7)]
[InlineData(true, 7)]
[InlineData(false, 8)]
[InlineData(true, 8)]
public async Task GZipStream_Read_RejectsTruncatedTrailer(bool useAsync, int missingBytes)
{
var data = CreateGZipWithExtraField(32);
using var input = new ShortReadStream(
new MemoryStream(data.Take(data.Length - missingBytes).ToArray())
);
await Assert.ThrowsAsync<ZlibException>(() =>
ReadPayload(input, "Stream", useAsync, CancellationToken.None)
);
Assert.False(input.IsDisposed);
}
[Theory]
[InlineData("Stream")]
[InlineData("Reader")]
[InlineData("Archive")]
public async Task GZip_ReadAsync_WithExtraField_HonorsCancellation(string api)
{
using var input = new ShortReadStream(new MemoryStream(CreateGZipWithExtraField(65535)));
using var cancellation = new CancellationTokenSource();
cancellation.Cancel();
await Assert.ThrowsAnyAsync<OperationCanceledException>(() =>
ReadPayload(input, api, true, cancellation.Token)
);
Assert.False(input.IsDisposed);
}
private static async Task<byte[]> ReadPayload(
Stream input,
string api,
bool useAsync,
CancellationToken cancellationToken
)
{
var options = ReaderOptions.ForExternalStream;
using var output = new MemoryStream();
switch (api)
{
case "Stream":
if (useAsync)
{
await using var gzip = new SharpCompress.Compressors.Deflate.GZipStream(
input,
SharpCompressionMode.Decompress,
options
);
await gzip.CopyToAsync(output, 81920, cancellationToken);
}
else
{
using var gzip = new SharpCompress.Compressors.Deflate.GZipStream(
input,
SharpCompressionMode.Decompress,
options
);
gzip.CopyTo(output);
}
break;
case "Reader":
if (useAsync)
{
await using var reader = await GZipReader.OpenAsyncReader(
input,
options,
cancellationToken
);
Assert.True(await reader.MoveToNextEntryAsync(cancellationToken));
await reader.WriteEntryToAsync(output, cancellationToken);
}
else
{
using var reader = GZipReader.OpenReader(input, options);
Assert.True(reader.MoveToNextEntry());
reader.WriteEntryTo(output);
}
break;
case "Archive":
if (useAsync)
{
await using var archive = await GZipArchive.OpenAsyncArchive(
input,
options,
cancellationToken
);
await foreach (
var entry in archive.EntriesAsync.WithCancellation(cancellationToken)
)
{
using var entryStream = await entry.OpenEntryStreamAsync(cancellationToken);
await entryStream.CopyToAsync(output, 81920, cancellationToken);
break;
}
}
else
{
using var archive = GZipArchive.OpenArchive(input, options);
using var entryStream = archive.Entries.Single().OpenEntryStream();
entryStream.CopyTo(output);
}
break;
default:
throw new ArgumentException("Unknown GZip API", nameof(api));
}
return output.ToArray();
}
[Theory]
[InlineData(32767)]
[InlineData(32768)]

View File

@@ -0,0 +1,28 @@
using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
namespace SharpCompress.Test.Mocks;
// Exercise legal partial reads through every stream read overload used by the parsers.
internal sealed class ShortReadStream(Stream stream, int maxReadSize = 1, bool seekable = true)
: TestStream(stream, read: true, write: false, seek: seekable)
{
public override int Read(byte[] buffer, int offset, int count) =>
base.Read(buffer, offset, Math.Min(count, maxReadSize));
public override Task<int> ReadAsync(
byte[] buffer,
int offset,
int count,
CancellationToken cancellationToken
) => base.ReadAsync(buffer, offset, Math.Min(count, maxReadSize), cancellationToken);
#if !LEGACY_DOTNET
public override ValueTask<int> ReadAsync(
Memory<byte> buffer,
CancellationToken cancellationToken = default
) => base.ReadAsync(buffer.Slice(0, Math.Min(buffer.Length, maxReadSize)), cancellationToken);
#endif
}

View File

@@ -1,5 +1,6 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using SharpCompress.Common.Rar;
using SharpCompress.Common.Rar.Headers;
using SharpCompress.IO;
@@ -10,6 +11,76 @@ namespace SharpCompress.Test.Rar;
public class CryptKey5Tests : TestBase
{
[Theory]
[InlineData(0)]
[InlineData(24)]
[InlineData(25)]
[InlineData(30)]
[InlineData(31)]
[InlineData(32)]
[InlineData(36)]
[InlineData(128)]
[InlineData(255)]
public async Task Rar5_CryptoInfo_Validates_Kdf_Count(int lg2Count)
{
// The KDF count is one raw byte, not a vint. Test the upper bound without deriving a key.
var data = new byte[35];
data[2] = (byte)lg2Count;
using var syncStream = new MemoryStream(data);
using var asyncStream = new MemoryStream(data);
using var syncReader = new MarkingBinaryReader(syncStream);
using var asyncReader = new AsyncMarkingBinaryReader(asyncStream);
if (lg2Count <= 24)
{
Assert.Equal(lg2Count, Rar5CryptoInfo.Create(syncReader, true).LG2Count);
Assert.Equal(lg2Count, (await Rar5CryptoInfo.CreateAsync(asyncReader, true)).LG2Count);
}
else
{
Assert.Throws<SharpCompress.Common.CryptographicException>(() =>
Rar5CryptoInfo.Create(syncReader, true)
);
await Assert.ThrowsAsync<SharpCompress.Common.CryptographicException>(async () =>
await Rar5CryptoInfo.CreateAsync(asyncReader, true)
);
Assert.Equal(3, syncStream.Position);
Assert.Equal(3, asyncStream.Position);
}
}
[Theory]
[InlineData(-1)]
[InlineData(25)]
[InlineData(30)]
[InlineData(31)]
[InlineData(32)]
[InlineData(33)]
[InlineData(34)]
[InlineData(35)]
[InlineData(36)]
[InlineData(255)]
[InlineData(int.MaxValue)]
public void Rar5_KeyCache_Rejects_Invalid_Kdf_Count(int lg2Count)
{
var cache = new Rar5KeyCache();
Assert.Throws<SharpCompress.Common.CryptographicException>(() =>
cache.GetKeys("test", new byte[16], lg2Count)
);
}
[Fact]
public void Rar5_Key_Rejects_Mutated_Kdf_Count()
{
var info = ReadCryptoInfo();
var key = new CryptKey5("test", info);
using var transform = key.Transformer(info.Salt);
info.LG2Count = 32;
Assert.Throws<SharpCompress.Common.CryptographicException>(() =>
key.Transformer(info.Salt)
);
}
private Rar5CryptoInfo ReadCryptoInfo()
{
using var stream = File.OpenRead(

View File

@@ -9,6 +9,7 @@ using SharpCompress.Archives;
using SharpCompress.Archives.Rar;
using SharpCompress.Common;
using SharpCompress.Common.Rar;
using SharpCompress.Common.Rar.Headers;
using SharpCompress.Compressors.Rar;
using SharpCompress.IO;
using SharpCompress.Readers;
@@ -203,7 +204,61 @@ public class RarCommentTests : TestBase
Assert.True(stream.CanRead);
}
public static IEnumerable<object[]> UnsupportedCommentMethods()
public static IEnumerable<object[]> TruncatedEncryptedComments()
{
foreach (var useAsync in new[] { false, true })
{
foreach (var useReader in new[] { false, true })
{
foreach (var length in new[] { 0, 15, 16, 31 })
{
yield return [useAsync, useReader, length];
}
}
}
}
[Theory]
[MemberData(nameof(TruncatedEncryptedComments))]
public async Task Rar5_Comment_Rejects_Truncated_Encrypted_Data(
bool useAsync,
bool useReader,
int length
)
{
var data = File.ReadAllBytes(
Path.Combine(TEST_ARCHIVES_PATH, "Rar5.comment.encrypted17.rar")
);
using var source = new MemoryStream(data);
var factory = new RarHeaderFactory(
StreamingMode.Seekable,
ReaderOptions.ForExternalStream.WithPassword("test")
);
var comment = factory
.ReadHeaders(source)
.OfType<FileHeader>()
.First(x => x.FileName == "CMT");
using var packed = comment.PackedStream!;
Assert.NotNull(comment.Rar5CryptoInfo);
Assert.True(comment.CompressedSize > length);
using var truncated = new MemoryStream(data.Take((int)source.Position + length).ToArray());
if (useAsync)
{
await Assert.ThrowsAsync<IncompleteArchiveException>(() =>
ReadCommentArchive(truncated, useAsync, useReader, "test")
);
}
else
{
await Assert.ThrowsAsync<InvalidFormatException>(() =>
ReadCommentArchive(truncated, useAsync, useReader, "test")
);
}
Assert.True(truncated.CanRead);
}
public static IEnumerable<object[]> NonStoredCommentMethods()
{
foreach (var useAsync in new[] { false, true })
{
@@ -218,8 +273,12 @@ public class RarCommentTests : TestBase
}
[Theory]
[MemberData(nameof(UnsupportedCommentMethods))]
public async Task Rar5_Comment_Skips_Nonzero_Methods(int method, bool useAsync, bool useReader)
[MemberData(nameof(NonStoredCommentMethods))]
public async Task Rar5_Comment_Skips_NonStored_Methods(
int method,
bool useAsync,
bool useReader
)
{
// Ignored comments must not allocate from their unpacked size or disturb the next entry.
using var stream = CreateCommentArchive(int.MaxValue, method, [1, 2, 3]);
@@ -230,10 +289,11 @@ public class RarCommentTests : TestBase
private static async Task<string?> ReadCommentArchive(
Stream stream,
bool useAsync,
bool useReader
bool useReader,
string? password = null
)
{
var options = ReaderOptions.ForExternalStream;
var options = ReaderOptions.ForExternalStream with { Password = password };
if (useReader)
{
using var forwardOnly = new ForwardOnlyStream(

View File

@@ -0,0 +1,120 @@
using System;
using System.IO;
using System.Linq;
using System.Security.Cryptography;
using System.Threading;
using System.Threading.Tasks;
using SharpCompress.Common;
using SharpCompress.Common.Rar;
using SharpCompress.IO;
using SharpCompress.Test.Mocks;
using Xunit;
namespace SharpCompress.Test.Rar;
public class RarCryptoWrapperTests
{
private static readonly byte[] Plaintext = Enumerable
.Range(0, 32)
.Select(x => (byte)x)
.ToArray();
[Theory]
[InlineData(false, 1)]
[InlineData(true, 1)]
[InlineData(false, 7)]
[InlineData(true, 7)]
[InlineData(false, 17)]
[InlineData(true, 17)]
[InlineData(false, 32)]
[InlineData(true, 32)]
public async Task Rar5_Crypto_Reads_Fragmented_Blocks_And_Buffered_Plaintext(
bool useAsync,
int readSize
)
{
var info = CreateCryptoInfo();
using var input = new ShortReadStream(new MemoryStream(Encrypt(info)));
using var asyncOnly = new AsyncOnlyStream(input, disposeStream: false);
using var crypto = new RarCryptoWrapper(
useAsync ? asyncOnly : input,
info.Salt,
new CryptKey5("test", info)
);
var result = new byte[Plaintext.Length];
var offset = 0;
while (offset < result.Length)
{
var count = Math.Min(readSize, result.Length - offset);
var read = useAsync
? await crypto.ReadAsync(result, offset, count, CancellationToken.None)
: crypto.Read(result, offset, count);
Assert.Equal(count, read);
offset += read;
}
Assert.Equal(Plaintext, result);
Assert.Equal(0, crypto.Read(result, 0, 0));
Assert.False(input.IsDisposed);
}
[Theory]
[InlineData(0)]
[InlineData(1)]
[InlineData(15)]
[InlineData(16)]
[InlineData(17)]
[InlineData(31)]
public async Task Rar5_Crypto_Rejects_Truncated_Ciphertext(int length)
{
var info = CreateCryptoInfo();
var data = Encrypt(info).Take(length).ToArray();
using var syncInput = new ShortReadStream(new MemoryStream(data));
using var asyncInput = new ShortReadStream(new MemoryStream(data));
using var syncCrypto = new RarCryptoWrapper(
syncInput,
info.Salt,
new CryptKey5("test", info)
);
using var asyncCrypto = new RarCryptoWrapper(
asyncInput,
info.Salt,
new CryptKey5("test", info)
);
Assert.Throws<InvalidFormatException>(() => syncCrypto.Read(new byte[32], 0, 32));
await Assert.ThrowsAsync<IncompleteArchiveException>(async () =>
await asyncCrypto.ReadAsync(new byte[32], 0, 32, CancellationToken.None)
);
}
[Fact]
public async Task Rar5_Crypto_Async_Cancellation()
{
var info = CreateCryptoInfo();
using var input = new MemoryStream(Encrypt(info));
using var crypto = new RarCryptoWrapper(input, info.Salt, new CryptKey5("test", info));
using var cancellation = new CancellationTokenSource();
cancellation.Cancel();
await Assert.ThrowsAnyAsync<OperationCanceledException>(async () =>
await crypto.ReadAsync(new byte[32], 0, 32, cancellation.Token)
);
Assert.Equal(0, input.Position);
}
private static Rar5CryptoInfo CreateCryptoInfo()
{
// Use a one-iteration KDF so these stream tests do not spend time deriving keys.
using var stream = new MemoryStream(new byte[35]);
using var reader = new MarkingBinaryReader(stream);
return Rar5CryptoInfo.Create(reader, true);
}
private static byte[] Encrypt(Rar5CryptoInfo info)
{
using var aes = Aes.Create();
aes.Key = CryptKey5.GenerateRarPBKDF2Key("test", info.Salt, 1)[0];
aes.IV = info.InitV;
aes.Padding = PaddingMode.None;
using var encryptor = aes.CreateEncryptor();
return encryptor.TransformFinalBlock(Plaintext, 0, Plaintext.Length);
}
}

View File

@@ -159,6 +159,72 @@ public class ZipArchiveTests : ArchiveTests
Assert.Equal(new[] { zip, z01 }, ArchiveFactory.GetFileParts(zip));
}
[Theory]
[InlineData("MULTI.ZIP", "MULTI.Z01", "Infozip.nocomp.multi")]
[InlineData("MULTI.ZiP", "Multi.z01", "Infozip.nocomp.multi")]
[InlineData("MULTI.ZIPX", "MULTI.ZX01", "WinZip26.nocomp.multi")]
[InlineData("MULTI.ZiPx", "Multi.zX01", "WinZip26.nocomp.multi")]
public void Zip_GetParts_MixedCaseCompanions(
string archiveName,
string partName,
string fixture
)
{
var zip = Path.Combine(SCRATCH2_FILES_PATH, archiveName);
var part = Path.Combine(SCRATCH2_FILES_PATH, partName);
var isZipX = archiveName.EndsWith("zipx", StringComparison.OrdinalIgnoreCase);
File.Copy(Path.Combine(TEST_ARCHIVES_PATH, fixture + (isZipX ? ".zipx" : ".zip")), zip);
File.Copy(Path.Combine(TEST_ARCHIVES_PATH, fixture + (isZipX ? ".zx01" : ".z01")), part);
var parts = ArchiveFactory.GetFileParts(zip).ToArray();
Assert.Equal(2, parts.Length);
Assert.Equal(zip, parts[0]);
Assert.True(string.Equals(part, parts[1], StringComparison.OrdinalIgnoreCase));
using var archive = ZipArchive.OpenArchive(zip);
Assert.NotEmpty(archive.Entries);
}
[Fact]
public void Zip_GetParts_RejectsAmbiguousCaseInsensitiveMatches()
{
var zip = Path.Combine(SCRATCH2_FILES_PATH, "MULTI.ZIP");
File.WriteAllBytes(zip, []);
File.WriteAllBytes(Path.Combine(SCRATCH2_FILES_PATH, "Multi.Z01"), []);
var other = Path.Combine(SCRATCH2_FILES_PATH, "multi.Z01");
if (File.Exists(other))
{
// Case-insensitive filesystems cannot represent an ambiguous pair.
return;
}
File.WriteAllBytes(other, []);
Assert.Throws<InvalidFormatException>(() => ArchiveFactory.GetFileParts(zip).ToArray());
}
[Fact]
public void Zip_GetParts_PrefersExactMatch()
{
var zip = Path.Combine(SCRATCH2_FILES_PATH, "MULTI.ZIP");
var part = Path.Combine(SCRATCH2_FILES_PATH, "MULTI.z01");
File.WriteAllBytes(zip, []);
File.WriteAllBytes(part, []);
var other = Path.Combine(SCRATCH2_FILES_PATH, "Multi.Z01");
if (!File.Exists(other))
{
File.WriteAllBytes(other, []);
}
Assert.Equal(new[] { zip, part }, ArchiveFactory.GetFileParts(zip));
}
[Fact]
public void Zip_GetParts_AllowsMissingCompanion()
{
var zip = Path.Combine(SCRATCH2_FILES_PATH, "MULTI.ZIP");
File.WriteAllBytes(zip, []);
Assert.Equal(new[] { zip }, ArchiveFactory.GetFileParts(zip));
}
[Fact]
public void Zip_GetPartsSplit() =>
//uses first part to search for all parts and compares against this array