From 46ba719daa21a9e4826fbc51a52c0da3284c46be Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 9 Nov 2026 11:28:55 +0000 Subject: [PATCH 1/2] Reject negative TAR entry sizes and overflowing seek offsets --- .../Common/Tar/Headers/TarHeader.cs | 13 +- .../Common/Tar/TarHeaderFactory.Async.cs | 4 +- .../Common/Tar/TarHeaderFactory.cs | 6 +- .../Tar/TarSizeValidationTests.cs | 271 ++++++++++++++++++ 4 files changed, 286 insertions(+), 8 deletions(-) create mode 100644 tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs diff --git a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs index 0239b118..27817f96 100644 --- a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs +++ b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs @@ -633,12 +633,19 @@ internal sealed partial class TarHeader private long ReadSize(byte[] buffer) { - if ((buffer[124] & 0x80) == 0x80) // if size in binary + var size = + (buffer[124] & 0x80) == 0x80 // if size in binary + ? BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(0x80)) + : ReadAsciiInt64Base8(buffer, 124, 11); + + // A negative payload size can rewind seekable archives to the same header. + // Validate here so ordinary entries and metadata use the same sync/async guard. + if (size < 0) { - return BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(0x80)); + throw new InvalidFormatException($"Tar entry size {size} cannot be negative."); } - return ReadAsciiInt64Base8(buffer, 124, 11); + return size; } 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..1628b95a 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: @@ -64,6 +64,6 @@ internal static partial class TarHeaderFactory { return size; } - return 512 - zeros + size; + return checked(512 - zeros + size); } } diff --git a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs new file mode 100644 index 00000000..93510361 --- /dev/null +++ b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs @@ -0,0 +1,271 @@ +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(-1L, true)] + [InlineData(-512L, true)] + [InlineData(-1024L, true)] + [InlineData(long.MinValue, true)] + public async Task Tar_Header_Rejects_Negative_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(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); + var checksum = TarHeader.RecalculateChecksum(header); + Encoding + .ASCII.GetBytes(Convert.ToString(checksum, 8).PadLeft(6, '0') + "\0 ") + .CopyTo(header, 148); + return header; + } +} From 30586fdcc5a28631ddb271253b09af98fc68dfb7 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 9 Nov 2026 11:34:11 +0000 Subject: [PATCH 2/2] Validate full-width TAR binary sizes Combine the decoder and padding size bounds from advisory PR #1 with the checked seek offsets and sync/async regression coverage in PR #2. Co-authored-by: 4nonym0us <2701409+4nonym0us@users.noreply.github.com> --- .../Common/Tar/Headers/TarHeader.cs | 26 +++++-- .../Common/Tar/TarHeaderFactory.cs | 9 ++- .../Tar/TarSizeValidationTests.cs | 74 ++++++++++++++++++- 3 files changed, 97 insertions(+), 12 deletions(-) diff --git a/src/SharpCompress/Common/Tar/Headers/TarHeader.cs b/src/SharpCompress/Common/Tar/Headers/TarHeader.cs index 27817f96..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,23 +632,36 @@ 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) { var size = - (buffer[124] & 0x80) == 0x80 // if size in binary - ? BinaryPrimitives.ReadInt64BigEndian(buffer.AsSpan(0x80)) + (buffer[124] & 0x80) != 0 + ? ReadBase256Size(buffer) : ReadAsciiInt64Base8(buffer, 124, 11); - // A negative payload size can rewind seekable archives to the same header. + // 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 < 0) + if (size is < 0 or > MAX_ENTRY_SIZE) { - throw new InvalidFormatException($"Tar entry size {size} cannot be negative."); + throw new InvalidFormatException($"Invalid TAR entry size: {size}."); } 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) { var buffer = reader.ReadBytes(BLOCK_SIZE); diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs index 1628b95a..e72b23c1 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs @@ -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 checked(512 - zeros + size); + + return (size + 511) & ~511L; } } diff --git a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs index 93510361..a8e8c6f6 100644 --- a/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs +++ b/tests/SharpCompress.Test/Tar/TarSizeValidationTests.cs @@ -21,11 +21,13 @@ public class TarSizeValidationTests [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_Negative_Size(long size, bool negativeMarker) + 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); @@ -41,6 +43,69 @@ public class TarSizeValidationTests ); } + [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)] @@ -262,10 +327,15 @@ public class TarSizeValidationTests 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); - return header; } }