Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TestArchive doesn't handle invalid offsets correctly #856

Open
adamkadzban opened this issue Dec 14, 2023 · 2 comments
Open

TestArchive doesn't handle invalid offsets correctly #856

adamkadzban opened this issue Dec 14, 2023 · 2 comments
Labels
bug zip Related to ZIP file format

Comments

@adamkadzban
Copy link

adamkadzban commented Dec 14, 2023

Describe the bug

Entry header offsets are not checked for validity before trying to seek to them in the stream. Invalid offsets (i.e. <0) cause InvalidArgumentExceptions to be thrown from inside System.IO code, which are not caught in the existing ZipException handlers. They bubble up and cause the entire archive to fail validation with a cryptic "The parameter is incorrect - {filename}" message.

Reproduction Code

using ICSharpCode.SharpZipLib.Zip;

namespace TestArchive
{
    internal class Program
    {
        static void Main(string[] args)
        {
            string zipPath = @"C:\temp\test.zip";
            using var stream = System.IO.File.OpenRead(zipPath);
            using var zip = new ZipFile(stream);
            bool isValid = zip.TestArchive(false, TestStrategy.FindAllErrors, ZipTestResultHandler); 
            if (!isValid)
            {
                throw new ArgumentException("Zip file is invalid!");
            }
        }

        static void ZipTestResultHandler(TestStatus status, string message)
        {
            if (status.Entry != null && !status.EntryValid && !string.IsNullOrEmpty(message))
            {
                Console.WriteLine(status.Entry.ZipFileIndex + ": " + status.Entry.Name);
                Console.WriteLine(message);
            }
        }
    }
}

Steps to reproduce

  1. Get a zip file with an entry whose Offset is negative (this is generally an invalid state and I don't know how to create one like this)
  2. Call TestArchive() and pass in a ZipTestResultHandler to get the test results

Expected behavior

I expect each individual entry with an invalid offset/header to be reported on (assuming TestStrategy.FindAllErrors is specified)

Operating System

Windows

Framework Version

Other

Tags

ZIP

Additional context

entry.Offset not being checked for > 0: https://github.com/icsharpcode/SharpZipLib/blob/master/src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs#L1184

Where the InvalidArgumentException from deep in System.IO gets caught outside the while loop (additional entries are not tested): https://github.com/icsharpcode/SharpZipLib/blob/master/src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs#L1144

@github-actions github-actions bot added the zip Related to ZIP file format label Dec 14, 2023
@SourceproStudio
Copy link

SourceproStudio commented Dec 14, 2023 via email

@adamkadzban
Copy link
Author

I have a proposed/local fix for this, let me know if you want me to open up a Pull Request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug zip Related to ZIP file format
Projects
None yet
Development

No branches or pull requests

2 participants