starred/sharpcompress
1
0
Fork 0
mirror of https://github.com/adamhathcock/sharpcompress.git synced 2026-02-04 05:25:00 +00:00
Code Issues 975 Packages Projects Releases 20 Wiki Activity

[PR #1024] [MERGED] Fix memory exhaustion in TAR header auto-detection #1447

New Issue
Closed
opened 2026-01-29 22:20:37 +00:00 by claunia · 0 comments
claunia commented 2026-01-29 22:20:37 +00:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/adamhathcock/sharpcompress/pull/1024
Author: @Copilot
Created: 11/19/2025
Status: ✅ Merged
Merged: 11/19/2025
Merged by: @adamhathcock

Base: master ← Head: copilot/fix-memory-exhaustion-bug

📝 Commits (2)

  • 0698031 Initial plan
  • 7b06652 Add validation to prevent memory exhaustion in TAR long name headers

📊 Changes

3 files changed (+69 additions, -1 deletions)

View changed files

📝 .gitignore (+2 -1)
📝 src/SharpCompress/Common/Tar/Headers/TarHeader.cs (+13 -0)
📝 tests/SharpCompress.Test/Tar/TarReaderTests.cs (+54 -0)

📄 Description

During auto-detection without extension hints, random bytes in compressed files (e.g., tar.lz) can be misinterpreted as TAR LongName/LongLink headers with multi-gigabyte sizes, causing memory exhaustion.

Changes

  • Added size validation in TarHeader.ReadLongName()

    • Introduced MAX_LONG_NAME_SIZE constant (32KB) - covers real-world path limits
    • Validates size before allocation: if (size < 0 || size > MAX_LONG_NAME_SIZE)
    • Throws InvalidFormatException (caught by IsTarFile, returns false, auto-detection continues)

  • Added regression test

    • Tar_Malformed_LongName_Excessive_Size creates malformed header with 8GB size
    • Verifies graceful failure instead of memory exhaustion

Example

// Before: Could attempt to allocate 8GB+ when detecting tar.lz
using var stream = File.OpenRead("archive.tar.lz");
using var reader = ReaderFactory.Open(stream);  // OutOfMemoryException

// After: Validation prevents excessive allocation, detection succeeds
using var stream = File.OpenRead("archive.tar.lz");
using var reader = ReaderFactory.Open(stream);  // Works correctly

All existing tests pass. No breaking changes.

Original prompt

This section details on the original issue you should resolve

<issue_title>Bug: Memory exhaustion when auto-detecting a specific tar.lz archive</issue_title>
<issue_description>### Summary

When reading a specific .tar.lz file without providing an extension hint, the library attempts to auto-detect the format. This process incorrectly identifies the file as a Tar archive with a LongLink header, leading to an attempt to allocate a massive amount of memory (e.g., 20GB). This causes the application to either crash or fail to open the archive. Standard compression utilities can open this same file without any issues.

The root cause appears to be a lack of validation in TarHeader.Read() and its helper methods.

Steps to Reproduce

  1. Use the library to open a specially crafted .tar.lz file.
  2. Do not specify ReaderOptions.ExtensionHint, forcing the library to auto-detect the archive type.
  3. The library will fail to open the file after a massive memory spike.

Root Cause Analysis

The problem occurs because the auto-detection mechanism first tries to parse the file as a standard Tar archive. My file is a .tar.lz, but a byte at a specific offset is misinterpreted.

  1. In TarHeader.Read(), the code enters a loop to process headers.

    internal bool Read(BinaryReader reader)
{
    string? longName = null;
    string? longLinkName = null;
    var hasLongValue = true;
    byte[] buffer;
    EntryType entryType;

    do
    {
        buffer = ReadBlock(reader);

        if (buffer.Length == 0)
        {
            return false;
        }

        entryType = ReadEntryType(buffer);

        // In my file, the byte at offset 157 is misinterpreted as EntryType.LongLink
        if (entryType == EntryType.LongName)
        {
            longName = ReadLongName(reader, buffer); // <- THIS LINE
            continue;
        }
        else if (entryType == EntryType.LongLink)
        {
            longLinkName = ReadLongName(reader, buffer); // <- THIS LINE
            continue;
        }

        hasLongValue = false;
    } while (hasLongValue);
//...
}

  2. For my specific file, the byte at offset 157 (read as entryType) happens to match EntryType.LongLink. This triggers a call to TarHeader.ReadLongName().

  3. Inside ReadLongName(), the ReadSize(buffer) method calculates an extremely large value for nameLength based on the misinterpreted header data. The subsequent call to reader.ReadBytes(nameLength) attempts to allocate a massive array without any sanity checks.

    private string ReadLongName(BinaryReader reader, byte[] buffer)
{
    var size = ReadSize(buffer); // Calculates a huge size
    var nameLength = (int)size;
    var nameBytes = reader.ReadBytes(nameLength); // <- ATTEMPTS HUGE ALLOCATION
    var remainingBytesToRead = BLOCK_SIZE - (nameLength % BLOCK_SIZE);

    // ...
    return ArchiveEncoding.Decode(nameBytes, 0, nameBytes.Length).TrimNulls();
}

  4. The BinaryReader.ReadBytes() method directly allocates memory based on the provided count.

    public virtual byte[] ReadBytes(int count)
{
    ArgumentOutOfRangeException.ThrowIfNegative(count);
    ThrowIfDisposed();

    if (count == 0)
    {
        return Array.Empty<byte>();
    }

    byte[] result = new byte[count]; // <- HUGE MEMORY ALLOCATION HAPPENS HERE
    int numRead = _stream.ReadAtLeast(result, result.Length, throwOnEndOfStream: false);

    // ...
    return result;
}

Stream Corruption

After the Tar parsing attempt fails (likely due to an EndOfStreamException or I/O error from Stream.ReadAtLeast()), the underlying Stream or SharpCompressStream appears to be left in a corrupted state.

When the auto-detection logic proceeds to the correct tar.lz format, it fails to read the header correctly. For example, it does not see the "LZIP" magic bytes at the beginning of the stream, even though debugging shows the bytes are present in the buffer. This strongly suggests that the stream's internal position or state has been irrecoverably altered by the failed read attempt.

Workaround

The issue can be avoided by explicitly setting ReaderOptions.ExtensionHint to guide the parser. This skips the problematic Tar auto-detection step.

// Example workaround
var options = new ReaderOptions { ExtensionHint = "tar.lz" };
using (var archive = ArchiveFactory.Open(filePath, options))
{
    // ...
}

However, most users would expect the auto-detection to be robust and would not think to set this option unless they have investigated the source code.</issue_description>

Comments on the Issue (you are @copilot in this section)

@adamhathcock Please make a P...

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/adamhathcock/sharpcompress/pull/1024 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 11/19/2025 **Status:** ✅ Merged **Merged:** 11/19/2025 **Merged by:** [@adamhathcock](https://github.com/adamhathcock) **Base:** `master` ← **Head:** `copilot/fix-memory-exhaustion-bug` --- ### 📝 Commits (2) - [`0698031`](https://github.com/adamhathcock/sharpcompress/commit/0698031ed40c5b2c1d4daad1c94f2d1b9046cfa5) Initial plan - [`7b06652`](https://github.com/adamhathcock/sharpcompress/commit/7b06652bffd6568bdfef8023d5380d665e9087f2) Add validation to prevent memory exhaustion in TAR long name headers ### 📊 Changes **3 files changed** (+69 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `.gitignore` (+2 -1) 📝 `src/SharpCompress/Common/Tar/Headers/TarHeader.cs` (+13 -0) 📝 `tests/SharpCompress.Test/Tar/TarReaderTests.cs` (+54 -0) </details> ### 📄 Description During auto-detection without extension hints, random bytes in compressed files (e.g., tar.lz) can be misinterpreted as TAR `LongName`/`LongLink` headers with multi-gigabyte sizes, causing memory exhaustion. ## Changes - **Added size validation in `TarHeader.ReadLongName()`** - Introduced `MAX_LONG_NAME_SIZE` constant (32KB) - covers real-world path limits - Validates size before allocation: `if (size < 0 || size > MAX_LONG_NAME_SIZE)` - Throws `InvalidFormatException` (caught by `IsTarFile`, returns false, auto-detection continues) - **Added regression test** - `Tar_Malformed_LongName_Excessive_Size` creates malformed header with 8GB size - Verifies graceful failure instead of memory exhaustion ## Example ```csharp // Before: Could attempt to allocate 8GB+ when detecting tar.lz using var stream = File.OpenRead("archive.tar.lz"); using var reader = ReaderFactory.Open(stream); // OutOfMemoryException // After: Validation prevents excessive allocation, detection succeeds using var stream = File.OpenRead("archive.tar.lz"); using var reader = ReaderFactory.Open(stream); // Works correctly ``` All existing tests pass. No breaking changes. <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> ---- *This section details on the original issue you should resolve* <issue_title>Bug: Memory exhaustion when auto-detecting a specific tar.lz archive</issue_title> <issue_description>### Summary When reading a specific `.tar.lz` file without providing an extension hint, the library attempts to auto-detect the format. This process incorrectly identifies the file as a `Tar` archive with a `LongLink` header, leading to an attempt to allocate a massive amount of memory (e.g., 20GB). This causes the application to either crash or fail to open the archive. Standard compression utilities can open this same file without any issues. The root cause appears to be a lack of validation in `TarHeader.Read()` and its helper methods. ### Steps to Reproduce 1. Use the library to open a specially crafted `.tar.lz` file. 2. Do not specify `ReaderOptions.ExtensionHint`, forcing the library to auto-detect the archive type. 3. The library will fail to open the file after a massive memory spike. ### Root Cause Analysis The problem occurs because the auto-detection mechanism first tries to parse the file as a standard `Tar` archive. My file is a `.tar.lz`, but a byte at a specific offset is misinterpreted. 1. In `TarHeader.Read()`, the code enters a loop to process headers. ```csharp internal bool Read(BinaryReader reader) { string? longName = null; string? longLinkName = null; var hasLongValue = true; byte[] buffer; EntryType entryType; do { buffer = ReadBlock(reader); if (buffer.Length == 0) { return false; } entryType = ReadEntryType(buffer); // In my file, the byte at offset 157 is misinterpreted as EntryType.LongLink if (entryType == EntryType.LongName) { longName = ReadLongName(reader, buffer); // <- THIS LINE continue; } else if (entryType == EntryType.LongLink) { longLinkName = ReadLongName(reader, buffer); // <- THIS LINE continue; } hasLongValue = false; } while (hasLongValue); //... } ``` 2. For my specific file, the byte at offset 157 (read as `entryType`) happens to match `EntryType.LongLink`. This triggers a call to `TarHeader.ReadLongName()`. 3. Inside `ReadLongName()`, the `ReadSize(buffer)` method calculates an extremely large value for `nameLength` based on the misinterpreted header data. The subsequent call to `reader.ReadBytes(nameLength)` attempts to allocate a massive array without any sanity checks. ```csharp private string ReadLongName(BinaryReader reader, byte[] buffer) { var size = ReadSize(buffer); // Calculates a huge size var nameLength = (int)size; var nameBytes = reader.ReadBytes(nameLength); // <- ATTEMPTS HUGE ALLOCATION var remainingBytesToRead = BLOCK_SIZE - (nameLength % BLOCK_SIZE); // ... return ArchiveEncoding.Decode(nameBytes, 0, nameBytes.Length).TrimNulls(); } ``` 4. The `BinaryReader.ReadBytes()` method directly allocates memory based on the provided `count`. ```csharp public virtual byte[] ReadBytes(int count) { ArgumentOutOfRangeException.ThrowIfNegative(count); ThrowIfDisposed(); if (count == 0) { return Array.Empty<byte>(); } byte[] result = new byte[count]; // <- HUGE MEMORY ALLOCATION HAPPENS HERE int numRead = _stream.ReadAtLeast(result, result.Length, throwOnEndOfStream: false); // ... return result; } ``` ### Stream Corruption After the `Tar` parsing attempt fails (likely due to an `EndOfStreamException` or I/O error from `Stream.ReadAtLeast()`), the underlying `Stream` or `SharpCompressStream` appears to be left in a corrupted state. When the auto-detection logic proceeds to the correct `tar.lz` format, it fails to read the header correctly. For example, it does not see the "LZIP" magic bytes at the beginning of the stream, even though debugging shows the bytes are present in the buffer. This strongly suggests that the stream's internal position or state has been irrecoverably altered by the failed read attempt. ### Workaround The issue can be avoided by explicitly setting `ReaderOptions.ExtensionHint` to guide the parser. This skips the problematic `Tar` auto-detection step. ```csharp // Example workaround var options = new ReaderOptions { ExtensionHint = "tar.lz" }; using (var archive = ArchiveFactory.Open(filePath, options)) { // ... } ``` However, most users would expect the auto-detection to be robust and would not think to set this option unless they have investigated the source code.</issue_description> ## Comments on the Issue (you are @copilot in this section) <comments> <comment_new><author>@adamhathcock</author><body> Please make a P... </details> - Fixes adamhathcock/sharpcompress#1021 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-29 22:20:37 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
duplicate
enhancement
feedback requested
not bug
pull-request
Mirrored from GitHub Pull Request
 question
up for grabs
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/sharpcompress#1447
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?