diff --git a/docs/USAGE.md b/docs/USAGE.md index 9666d43f..18249a87 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -472,3 +472,6 @@ await archive.WriteToDirectoryAsync( - Support for cancellation via CancellationToken - Better resource utilization in async/await contexts - Compatible with modern .NET async patterns +## Malformed input and memory use + +ZIP64 end-record extensible data is skipped with bounded buffering. Invalid record sizes and truncated extensible data are rejected rather than driving allocations from the declared size. diff --git a/src/SharpCompress/Common/Zip/Headers/Zip64DirectoryEndHeader.Async.cs b/src/SharpCompress/Common/Zip/Headers/Zip64DirectoryEndHeader.Async.cs index 9e510688..df9e7cbe 100644 --- a/src/SharpCompress/Common/Zip/Headers/Zip64DirectoryEndHeader.Async.cs +++ b/src/SharpCompress/Common/Zip/Headers/Zip64DirectoryEndHeader.Async.cs @@ -1,3 +1,4 @@ +using System; using System.IO; using System.Threading.Tasks; using SharpCompress.IO; @@ -8,7 +9,9 @@ internal partial class Zip64DirectoryEndHeader { internal override async ValueTask Read(AsyncBinaryReader reader) { - SizeOfDirectoryEndRecord = (long)await reader.ReadUInt64Async().ConfigureAwait(false); + SizeOfDirectoryEndRecord = ValidateRecordSize( + await reader.ReadUInt64Async().ConfigureAwait(false) + ); VersionMadeBy = await reader.ReadUInt16Async().ConfigureAwait(false); VersionNeededToExtract = await reader.ReadUInt16Async().ConfigureAwait(false); VolumeNumber = await reader.ReadUInt32Async().ConfigureAwait(false); @@ -18,10 +21,14 @@ internal partial class Zip64DirectoryEndHeader DirectorySize = (long)await reader.ReadUInt64Async().ConfigureAwait(false); DirectoryStartOffsetRelativeToDisk = (long) await reader.ReadUInt64Async().ConfigureAwait(false); - var size = (int)( - SizeOfDirectoryEndRecord - SIZE_OF_FIXED_HEADER_DATA_EXCEPT_SIGNATURE_AND_SIZE_FIELDS - ); - DataSector = new byte[size]; - await reader.ReadBytesAsync(DataSector, 0, size).ConfigureAwait(false); + var remaining = + SizeOfDirectoryEndRecord - SIZE_OF_FIXED_HEADER_DATA_EXCEPT_SIGNATURE_AND_SIZE_FIELDS; + var buffer = new byte[4096]; + while (remaining > 0) + { + var size = (int)Math.Min(buffer.Length, remaining); + await reader.ReadBytesAsync(buffer, 0, size).ConfigureAwait(false); + remaining -= size; + } } } diff --git a/src/SharpCompress/Common/Zip/Headers/Zip64DirectoryEndHeader.cs b/src/SharpCompress/Common/Zip/Headers/Zip64DirectoryEndHeader.cs index 3933b2e0..96d01637 100644 --- a/src/SharpCompress/Common/Zip/Headers/Zip64DirectoryEndHeader.cs +++ b/src/SharpCompress/Common/Zip/Headers/Zip64DirectoryEndHeader.cs @@ -1,3 +1,4 @@ +using System; using System.IO; namespace SharpCompress.Common.Zip.Headers; @@ -9,7 +10,7 @@ internal partial class Zip64DirectoryEndHeader : ZipHeader internal override void Read(BinaryReader reader) { - SizeOfDirectoryEndRecord = (long)reader.ReadUInt64(); + SizeOfDirectoryEndRecord = ValidateRecordSize(reader.ReadUInt64()); VersionMadeBy = reader.ReadUInt16(); VersionNeededToExtract = reader.ReadUInt16(); VolumeNumber = reader.ReadUInt32(); @@ -18,12 +19,31 @@ internal partial class Zip64DirectoryEndHeader : ZipHeader TotalNumberOfEntries = (long)reader.ReadUInt64(); DirectorySize = (long)reader.ReadUInt64(); DirectoryStartOffsetRelativeToDisk = (long)reader.ReadUInt64(); - DataSector = reader.ReadBytes( - (int)( - SizeOfDirectoryEndRecord - - SIZE_OF_FIXED_HEADER_DATA_EXCEPT_SIGNATURE_AND_SIZE_FIELDS - ) - ); + // Extensible data is not interpreted. Consume it without allocating from its declared size. + var remaining = + SizeOfDirectoryEndRecord - SIZE_OF_FIXED_HEADER_DATA_EXCEPT_SIGNATURE_AND_SIZE_FIELDS; + var buffer = new byte[4096]; + while (remaining > 0) + { + var read = reader.Read(buffer, 0, (int)Math.Min(buffer.Length, remaining)); + if (read == 0) + { + throw new IncompleteArchiveException("Truncated ZIP64 extensible data."); + } + remaining -= read; + } + } + + private static long ValidateRecordSize(ulong size) + { + if ( + size < SIZE_OF_FIXED_HEADER_DATA_EXCEPT_SIGNATURE_AND_SIZE_FIELDS + || size > long.MaxValue + ) + { + throw new InvalidFormatException("Invalid ZIP64 end record size."); + } + return (long)size; } private const int SIZE_OF_FIXED_HEADER_DATA_EXCEPT_SIGNATURE_AND_SIZE_FIELDS = 44; @@ -45,6 +65,4 @@ internal partial class Zip64DirectoryEndHeader : ZipHeader public long DirectorySize { get; private set; } public long DirectoryStartOffsetRelativeToDisk { get; private set; } - - public byte[]? DataSector { get; private set; } } diff --git a/tests/SharpCompress.Test/Zip/Zip64AllocationTests.cs b/tests/SharpCompress.Test/Zip/Zip64AllocationTests.cs new file mode 100644 index 00000000..48ed6024 --- /dev/null +++ b/tests/SharpCompress.Test/Zip/Zip64AllocationTests.cs @@ -0,0 +1,68 @@ +using System; +using System.IO; +using System.Threading.Tasks; +using SharpCompress.Common; +using SharpCompress.Common.Zip.Headers; +using SharpCompress.IO; +using Xunit; + +namespace SharpCompress.Test.Zip; + +public class Zip64AllocationTests +{ + [Theory] + [InlineData(0UL)] + [InlineData(43UL)] + [InlineData(ulong.MaxValue)] + public async Task Zip64_Rejects_Invalid_Record_Size(ulong size) + { + using var stream = CreateRecord(size, 0); + Assert.Throws(() => + new Zip64DirectoryEndHeader().Read(new BinaryReader(stream)) + ); + stream.Position = 0; + await Assert.ThrowsAsync(async () => + await new Zip64DirectoryEndHeader().Read(new AsyncBinaryReader(stream)) + ); + } + + [Theory] + [InlineData(44UL, 0)] + [InlineData(10044UL, 10000)] + public async Task Zip64_Skips_Extensible_Data_Exactly(ulong size, int payloadSize) + { + using var stream = CreateRecord(size, payloadSize); + new Zip64DirectoryEndHeader().Read(new BinaryReader(stream)); + Assert.Equal(stream.Length, stream.Position); + stream.Position = 0; + await new Zip64DirectoryEndHeader().Read(new AsyncBinaryReader(stream)); + Assert.Equal(stream.Length, stream.Position); + } + + [Theory] + [InlineData(2147483691UL)] + [InlineData(4294967340UL)] + public async Task Zip64_Rejects_Huge_Truncated_Extensible_Data(ulong size) + { + using var stream = CreateRecord(size, 1); + Assert.ThrowsAny(() => + new Zip64DirectoryEndHeader().Read(new BinaryReader(stream)) + ); + stream.Position = 0; + await Assert.ThrowsAnyAsync(async () => + await new Zip64DirectoryEndHeader().Read(new AsyncBinaryReader(stream)) + ); + } + + private static MemoryStream CreateRecord(ulong size, int payloadSize) + { + var stream = new MemoryStream(); + using (var writer = new BinaryWriter(stream, System.Text.Encoding.UTF8, true)) + { + writer.Write(size); + writer.Write(new byte[44 + payloadSize]); + } + stream.Position = 0; + return stream; + } +}