diff --git a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs index 0239b118..559a27bc 100644 --- a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs +++ b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs @@ -37,6 +37,7 @@ internal sealed partial class TarHeader internal IArchiveEncoding ArchiveEncoding { get; } internal const int BLOCK_SIZE = 512; + internal const long MAX_ENTRY_SIZE = long.MaxValue - (BLOCK_SIZE - 1); // Maximum size for long name/link headers to prevent memory exhaustion attacks // This is generous enough for most real-world scenarios (32KB) @@ -631,14 +632,34 @@ internal sealed partial class TarHeader private static EntryType ReadEntryType(byte[] buffer) => (EntryType)buffer[156]; - private long ReadSize(byte[] buffer) + private static long ReadSize(byte[] buffer) { - if ((buffer[124] & 0x80) == 0x80) // if size in binary + var size = + (buffer[124] & 0x80) != 0 + ? ReadBase256Size(buffer) + : ReadAsciiInt64Base8(buffer, 124, 11); + + // Negative sizes can rewind seekable archives; oversized sizes overflow padding. + // Validate here so ordinary entries and metadata use the same sync/async guard. + if (size is < 0 or > MAX_ENTRY_SIZE) { - return BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(0x80)); + throw new InvalidFormatException($"Invalid TAR entry size: {size}."); } - return ReadAsciiInt64Base8(buffer, 124, 11); + return size; + } + + private static long ReadBase256Size(byte[] buffer) + { + // Only the last eight bytes fit in a long. Do not silently truncate the + // remaining value bits, including the sign bits of GNU negative sizes. + var leadingBits = (buffer[124] & 0x7F) | buffer[125] | buffer[126] | buffer[127]; + if (leadingBits != 0) + { + throw new InvalidFormatException("TAR entry size does not fit in 64 bits."); + } + + return BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(128, 8)); } private static byte[] ReadBlock(BinaryReader reader) diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs index c011abd2..ed5a62de 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs @@ -38,8 +38,8 @@ internal static partial class TarHeaderFactory { header.DataStartPosition = stream.Position; - //skip to nearest 512 - stream.Position += PadTo512(header.Size); + // Reject padded sizes or end offsets that would wrap into a backward seek. + stream.Position = checked(stream.Position + PadTo512(header.Size)); } break; case StreamingMode.Streaming: diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs index c95efaef..e72b23c1 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs @@ -32,8 +32,8 @@ internal static partial class TarHeaderFactory { header.DataStartPosition = stream.Position; - //skip to nearest 512 - stream.Position += PadTo512(header.Size); + // Reject padded sizes or end offsets that would wrap into a backward seek. + stream.Position = checked(stream.Position + PadTo512(header.Size)); } break; case StreamingMode.Streaming: @@ -59,11 +59,12 @@ internal static partial class TarHeaderFactory private static long PadTo512(long size) { - var zeros = (int)(size % 512); - if (zeros == 0) + // PAX size overrides bypass ReadSize, so validate the effective size too. + if (size is < 0 or > TarHeader.MAX_ENTRY_SIZE) { - return size; + throw new InvalidFormatException($"Invalid TAR entry size: {size}."); } - return 512 - zeros + size; + + return (size + 511) & ~511L; } } diff --git a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs new file mode 100644 index 00000000..a8e8c6f6 --- /dev/null +++ b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs @@ -0,0 +1,341 @@ +using System; +using System.Buffers.Binary; +using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Common; +using SharpCompress.Common.Tar.Headers; +using SharpCompress.IO; +using SharpCompress.Readers; +using SharpCompress.Test.Mocks; +using Xunit; + +namespace SharpCompress.Test.Tar; + +public class TarSizeValidationTests +{ + [Theory] + [InlineData(-1L, false)] + [InlineData(-512L, false)] + [InlineData(-1024L, false)] + [InlineData(long.MinValue, false)] + [InlineData(long.MaxValue, false)] + [InlineData(TarHeader.MAX_ENTRY_SIZE + 1, false)] + [InlineData(-1L, true)] + [InlineData(-512L, true)] + [InlineData(-1024L, true)] + [InlineData(long.MinValue, true)] + public async Task Tar_Header_Rejects_Invalid_Size(long size, bool negativeMarker) + { + using var stream = new MemoryStream(CreateHeader(size, negativeMarker)); + using var reader = new BinaryReader(stream, Encoding.UTF8, leaveOpen: true); + var header = new TarHeader(new ArchiveEncoding()); + + Assert.Throws(() => header.Read(reader)); + + stream.Position = 0; + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + using var asyncReader = new AsyncBinaryReader(asyncStream, leaveOpen: true); + await Assert.ThrowsAsync(async () => + await header.ReadAsync(asyncReader, cancellationToken: CancellationToken.None) + ); + } + + [Theory] + [InlineData(TarHeader.MAX_ENTRY_SIZE - 1)] + [InlineData(TarHeader.MAX_ENTRY_SIZE)] + public async Task Tar_Header_Accepts_Maximum_Paddable_Size(long size) + { + using var stream = new MemoryStream(CreateHeader(size)); + using var reader = new BinaryReader(stream, Encoding.UTF8, leaveOpen: true); + var header = new TarHeader(new ArchiveEncoding()); + + Assert.True(header.Read(reader)); + Assert.Equal(size, header.Size); + + stream.Position = 0; + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + using var asyncReader = new AsyncBinaryReader(asyncStream, leaveOpen: true); + Assert.True(await header.ReadAsync(asyncReader, cancellationToken: CancellationToken.None)); + Assert.Equal(size, header.Size); + } + + [Theory] + [InlineData(124, false)] + [InlineData(125, false)] + [InlineData(126, false)] + [InlineData(127, false)] + [InlineData(124, true)] + [InlineData(125, true)] + [InlineData(126, true)] + [InlineData(127, true)] + public async Task Tar_ArchiveFactory_Rejects_Binary_Size_With_Ignored_High_Bits( + int offset, + bool useAsync + ) + { + var header = CreateHeader(0); + // Keep the binary marker, but encode a size larger than a long can hold. + header[offset] |= 0x01; + WriteChecksum(header); + using var stream = new MemoryStream(header); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await Assert.ThrowsAsync(async () => + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + }); + } + else + { + Assert.Throws(() => + { + using var archive = ArchiveFactory.OpenArchive( + stream, + ReaderOptions.ForExternalStream + ); + }); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Tar_ArchiveFactory_Rejects_Negative_First_Entry(bool useAsync) + { + // The advisory payload is just one checksum-valid header with size -512. + using var stream = new MemoryStream(CreateHeader(-512)); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await Assert.ThrowsAsync(async () => + { + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + }); + } + else + { + Assert.Throws(() => + { + using var archive = ArchiveFactory.OpenArchive( + stream, + ReaderOptions.ForExternalStream + ); + }); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(-1L, false)] + [InlineData(-512L, false)] + [InlineData(-1024L, false)] + [InlineData(long.MinValue, false)] + [InlineData(long.MaxValue, false)] + [InlineData(long.MaxValue - 511, false)] + [InlineData(-1L, true)] + [InlineData(-512L, true)] + [InlineData(-1024L, true)] + [InlineData(long.MinValue, true)] + [InlineData(long.MaxValue, true)] + [InlineData(long.MaxValue - 511, true)] + public async Task Tar_Archive_Rejects_Invalid_Entry_Size(long size, bool useAsync) + { + using var stream = CreateArchive(size); + // A valid first entry bypasses format detection so enumeration must validate + // the next header. Only advance twice to keep regressions from looping. + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + await using var entries = archive.EntriesAsync.GetAsyncEnumerator( + CancellationToken.None + ); + Assert.True(await entries.MoveNextAsync()); + await Assert.ThrowsAsync(async () => + await entries.MoveNextAsync() + ); + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream, ReaderOptions.ForExternalStream); + using var entries = archive.Entries.GetEnumerator(); + Assert.True(entries.MoveNext()); + Assert.Throws(() => entries.MoveNext()); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Tar_Reader_Rejects_Negative_Entry_Size(bool useAsync) + { + using var stream = new ForwardOnlyStream(CreateArchive(-512)); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var reader = await ReaderFactory.OpenAsyncReader( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + await Assert.ThrowsAsync(async () => + await reader.MoveToNextEntryAsync(CancellationToken.None) + ); + } + else + { + using var reader = ReaderFactory.OpenReader(stream, ReaderOptions.ForExternalStream); + Assert.True(reader.MoveToNextEntry()); + Assert.Throws(() => reader.MoveToNextEntry()); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(0, false)] + [InlineData(1, false)] + [InlineData(511, false)] + [InlineData(512, false)] + [InlineData(513, false)] + [InlineData(0, true)] + [InlineData(1, true)] + [InlineData(511, true)] + [InlineData(512, true)] + [InlineData(513, true)] + public async Task Tar_Archive_Preserves_Nonnegative_Binary_Sizes(int size, bool useAsync) + { + using var stream = CreateArchive(size); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var archive = await ArchiveFactory.OpenAsyncArchive( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + await using var entries = archive.EntriesAsync.GetAsyncEnumerator( + CancellationToken.None + ); + Assert.True(await entries.MoveNextAsync()); + Assert.Equal(0, entries.Current.Size); + Assert.True(await entries.MoveNextAsync()); + Assert.Equal(size, entries.Current.Size); + Assert.False(await entries.MoveNextAsync()); + } + else + { + using var archive = ArchiveFactory.OpenArchive(stream, ReaderOptions.ForExternalStream); + using var entries = archive.Entries.GetEnumerator(); + Assert.True(entries.MoveNext()); + Assert.Equal(0, entries.Current.Size); + Assert.True(entries.MoveNext()); + Assert.Equal(size, entries.Current.Size); + Assert.False(entries.MoveNext()); + } + Assert.True(stream.CanRead); + } + + [Theory] + [InlineData(0, false)] + [InlineData(1, false)] + [InlineData(511, false)] + [InlineData(512, false)] + [InlineData(513, false)] + [InlineData(0, true)] + [InlineData(1, true)] + [InlineData(511, true)] + [InlineData(512, true)] + [InlineData(513, true)] + public async Task Tar_Reader_Preserves_Nonnegative_Binary_Sizes(int size, bool useAsync) + { + using var stream = new ForwardOnlyStream(CreateArchive(size)); + if (useAsync) + { + using var asyncStream = new AsyncOnlyStream(stream, disposeStream: false); + await using var reader = await ReaderFactory.OpenAsyncReader( + asyncStream, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + Assert.Equal(0, reader.Entry.Size); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + Assert.Equal(size, reader.Entry.Size); + Assert.False(await reader.MoveToNextEntryAsync(CancellationToken.None)); + } + else + { + using var reader = ReaderFactory.OpenReader(stream, ReaderOptions.ForExternalStream); + Assert.True(reader.MoveToNextEntry()); + Assert.Equal(0, reader.Entry.Size); + Assert.True(reader.MoveToNextEntry()); + Assert.Equal(size, reader.Entry.Size); + Assert.False(reader.MoveToNextEntry()); + } + Assert.True(stream.CanRead); + } + + private static MemoryStream CreateArchive(long size) + { + var stream = new MemoryStream(); + var firstHeader = CreateHeader(0); + stream.Write(firstHeader, 0, firstHeader.Length); + var header = CreateHeader(size); + stream.Write(header, 0, header.Length); + // Invalid sizes need no payload, and must never cause large test allocations. + if (size >= 0 && size <= 1024) + { + stream.SetLength(stream.Length + ((size + 511) / 512 * 512)); + } + stream.SetLength(stream.Length + 1024); + stream.Position = 0; + return stream; + } + + private static byte[] CreateHeader(long size, bool negativeMarker = false) + { + var header = new byte[512]; + header[0] = (byte)'x'; + header[124] = 0x80; + if (negativeMarker) + { + // Cover GNU negative encoding as well as the advisory's 0x80 prefix. + for (var i = 124; i < 128; i++) + { + header[i] = 0xff; + } + } + BinaryPrimitives.WriteInt64BigEndian(header.AsSpan(128, 8), size); + header[156] = (byte)'0'; + Encoding.ASCII.GetBytes("ustar\0" + "00").CopyTo(header, 257); + WriteChecksum(header); + return header; + } + + private static void WriteChecksum(byte[] header) + { + var checksum = TarHeader.RecalculateChecksum(header); + Encoding + .ASCII.GetBytes(Convert.ToString(checksum, 8).PadLeft(6, '0') + "\0 ") + .CopyTo(header, 148); + } +}