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

SharpCompress generates Zips that don't pass System.IO.Packaging validation #122

New Issue
Closed
opened 2026-01-29 22:06:55 +00:00 by claunia · 11 comments
claunia commented 2026-01-29 22:06:55 +00:00
Owner
Copy Link

Originally created by @anaisbetts on GitHub (Aug 23, 2016).

I'm working on switching Squirrel.Windows to SharpCompress in https://github.com/Squirrel/Squirrel.Windows/pull/803, and one thing I'm seeing is that files compressed with SharpCompress and then later opened via NuGet's code (which uses WPF's System.IO.Packaging code) blow up here:

    WindowsBase.dll!MS.Internal.IO.Zip.ZipIOLocalFileBlock.Validate(string fileName, MS.Internal.IO.Zip.ZipIOCentralDirectoryBlock centralDir, MS.Internal.IO.Zip.ZipIOCentralDirectoryFileHeader centralDirFileHeader) Unknown
    WindowsBase.dll!MS.Internal.IO.Zip.ZipIOLocalFileBlock.ParseRecord(System.IO.BinaryReader reader, string fileName, long position, MS.Internal.IO.Zip.ZipIOCentralDirectoryBlock centralDir, MS.Internal.IO.Zip.ZipIOCentralDirectoryFileHeader centralDirFileHeader)    Unknown
    WindowsBase.dll!MS.Internal.IO.Zip.ZipIOLocalFileBlock.SeekableLoad(MS.Internal.IO.Zip.ZipIOBlockManager blockManager, string fileName) Unknown
    WindowsBase.dll!MS.Internal.IO.Zip.ZipIOBlockManager.LoadLocalFileBlock(string zipFileName) Unknown
    WindowsBase.dll!MS.Internal.IO.Zip.ZipArchive.GetFile(string zipFileName)   Unknown
    WindowsBase.dll!MS.Internal.IO.Zip.ZipArchive.GetFiles()    Unknown
    WindowsBase.dll!System.IO.Packaging.ZipPackage.ContentTypeHelper.ContentTypeHelper(MS.Internal.IO.Zip.ZipArchive zipArchive, System.IO.Packaging.ZipPackage.IgnoredItemHelper ignoredItemHelper)    Unknown
    WindowsBase.dll!System.IO.Packaging.ZipPackage.ZipPackage(System.IO.Stream s, System.IO.FileMode mode, System.IO.FileAccess access, bool streaming) Unknown
    WindowsBase.dll!System.IO.Packaging.Package.Open(System.IO.Stream stream, System.IO.FileMode packageMode, System.IO.FileAccess packageAccess, bool streaming)   Unknown
    WindowsBase.dll!System.IO.Packaging.Package.Open(System.IO.Stream stream)   Unknown
>   NuGet.Squirrel.dll!NuGet.ZipPackage.EnsureManifest() Line 142   C#
    NuGet.Squirrel.dll!NuGet.ZipPackage.ZipPackage(string filePath, bool enableCaching) Line 53 C#

http://referencesource.microsoft.com/#WindowsBase/Base/MS/Internal/IO/Zip/ZipIOLocalFileBlock.cs,788

        private void Validate(string fileName, 
            ZipIOCentralDirectoryBlock centralDir,
            ZipIOCentralDirectoryFileHeader centralDirFileHeader)
        {
            // check that name matches parameter in a case sensitive culture neutral way
            if (0 != String.CompareOrdinal(_localFileHeader.FileName, fileName))
            {
                throw new FileFormatException(SR.Get(SRID.CorruptedData));
            }

            // compare compressed and uncompressed sizes, crc from central directory 
            if ((VersionNeededToExtract != centralDirFileHeader.VersionNeededToExtract) ||
                (GeneralPurposeBitFlag != centralDirFileHeader.GeneralPurposeBitFlag) ||
                (CompressedSize != centralDirFileHeader.CompressedSize) ||
                (UncompressedSize != centralDirFileHeader.UncompressedSize) ||
                (CompressionMethod != centralDirFileHeader.CompressionMethod) ||
                (Crc32 != centralDirFileHeader.Crc32))
            {
                throw new FileFormatException(SR.Get(SRID.CorruptedData));
            }

            // check for read into central directory (which would indicate file corruption)
            if (Offset + Size > centralDir.Offset)
                throw new FileFormatException(SR.Get(SRID.CorruptedData));

            // No CRC check here
            // delay validating the actual CRC until it is possible to do so without additional read operations
            // This is only for non-streaming mode (at this point we only support creation not consumption)
            // This is to avoid the forced reading of entire stream just for CRC check
            // CRC check is delegated  to ProgressiveCrcCalculatingStream and CRC is only validated
            //  once calculated CRC is available. This implies that CRC check operation is not
            //  guaranteed to be performed
        }

If you want a live repro, clone that PR and run the CreateFullPackagesFromDeltaSmokeTest test, or if you want a sample file to check out, https://cl.ly/0W3D3V1x0V2J

Originally created by @anaisbetts on GitHub (Aug 23, 2016). I'm working on switching Squirrel.Windows to SharpCompress in https://github.com/Squirrel/Squirrel.Windows/pull/803, and one thing I'm seeing is that files compressed with SharpCompress and then later opened via NuGet's code (which uses WPF's System.IO.Packaging code) blow up here: ``` WindowsBase.dll!MS.Internal.IO.Zip.ZipIOLocalFileBlock.Validate(string fileName, MS.Internal.IO.Zip.ZipIOCentralDirectoryBlock centralDir, MS.Internal.IO.Zip.ZipIOCentralDirectoryFileHeader centralDirFileHeader) Unknown WindowsBase.dll!MS.Internal.IO.Zip.ZipIOLocalFileBlock.ParseRecord(System.IO.BinaryReader reader, string fileName, long position, MS.Internal.IO.Zip.ZipIOCentralDirectoryBlock centralDir, MS.Internal.IO.Zip.ZipIOCentralDirectoryFileHeader centralDirFileHeader) Unknown WindowsBase.dll!MS.Internal.IO.Zip.ZipIOLocalFileBlock.SeekableLoad(MS.Internal.IO.Zip.ZipIOBlockManager blockManager, string fileName) Unknown WindowsBase.dll!MS.Internal.IO.Zip.ZipIOBlockManager.LoadLocalFileBlock(string zipFileName) Unknown WindowsBase.dll!MS.Internal.IO.Zip.ZipArchive.GetFile(string zipFileName) Unknown WindowsBase.dll!MS.Internal.IO.Zip.ZipArchive.GetFiles() Unknown WindowsBase.dll!System.IO.Packaging.ZipPackage.ContentTypeHelper.ContentTypeHelper(MS.Internal.IO.Zip.ZipArchive zipArchive, System.IO.Packaging.ZipPackage.IgnoredItemHelper ignoredItemHelper) Unknown WindowsBase.dll!System.IO.Packaging.ZipPackage.ZipPackage(System.IO.Stream s, System.IO.FileMode mode, System.IO.FileAccess access, bool streaming) Unknown WindowsBase.dll!System.IO.Packaging.Package.Open(System.IO.Stream stream, System.IO.FileMode packageMode, System.IO.FileAccess packageAccess, bool streaming) Unknown WindowsBase.dll!System.IO.Packaging.Package.Open(System.IO.Stream stream) Unknown > NuGet.Squirrel.dll!NuGet.ZipPackage.EnsureManifest() Line 142 C# NuGet.Squirrel.dll!NuGet.ZipPackage.ZipPackage(string filePath, bool enableCaching) Line 53 C# ``` http://referencesource.microsoft.com/#WindowsBase/Base/MS/Internal/IO/Zip/ZipIOLocalFileBlock.cs,788 ``` cs private void Validate(string fileName, ZipIOCentralDirectoryBlock centralDir, ZipIOCentralDirectoryFileHeader centralDirFileHeader) { // check that name matches parameter in a case sensitive culture neutral way if (0 != String.CompareOrdinal(_localFileHeader.FileName, fileName)) { throw new FileFormatException(SR.Get(SRID.CorruptedData)); } // compare compressed and uncompressed sizes, crc from central directory if ((VersionNeededToExtract != centralDirFileHeader.VersionNeededToExtract) || (GeneralPurposeBitFlag != centralDirFileHeader.GeneralPurposeBitFlag) || (CompressedSize != centralDirFileHeader.CompressedSize) || (UncompressedSize != centralDirFileHeader.UncompressedSize) || (CompressionMethod != centralDirFileHeader.CompressionMethod) || (Crc32 != centralDirFileHeader.Crc32)) { throw new FileFormatException(SR.Get(SRID.CorruptedData)); } // check for read into central directory (which would indicate file corruption) if (Offset + Size > centralDir.Offset) throw new FileFormatException(SR.Get(SRID.CorruptedData)); // No CRC check here // delay validating the actual CRC until it is possible to do so without additional read operations // This is only for non-streaming mode (at this point we only support creation not consumption) // This is to avoid the forced reading of entire stream just for CRC check // CRC check is delegated to ProgressiveCrcCalculatingStream and CRC is only validated // once calculated CRC is available. This implies that CRC check operation is not // guaranteed to be performed } ``` If you want a live repro, clone that PR and run the `CreateFullPackagesFromDeltaSmokeTest` test, or if you want a sample file to check out, https://cl.ly/0W3D3V1x0V2J
claunia commented 2026-01-29 22:06:56 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (Aug 23, 2016):

Did you create the zip file with ZipWriter? It might not like the streaming mode entries as there's a trailing header behind the file bytes to record the actual size and whatnot.

Knowing which way you created the file will help the most. It could be a bug in SharpCompress or it could be that the packaging API doesn't like streaming created zip files that are to spec :) Does something like WinZip or WinRar open the file?

I'm about to go on vacation for a week so I'm not sure when I'll get to take a look.

@adamhathcock commented on GitHub (Aug 23, 2016): Did you create the zip file with ZipWriter? It might not like the streaming mode entries as there's a trailing header behind the file bytes to record the actual size and whatnot. Knowing which way you created the file will help the most. It could be a bug in SharpCompress or it could be that the packaging API doesn't like streaming created zip files that are to spec :) Does something like WinZip or WinRar open the file? I'm about to go on vacation for a week so I'm not sure when I'll get to take a look.
claunia commented 2026-01-29 22:06:56 +00:00
Author
Owner
Copy Link

@anaisbetts commented on GitHub (Aug 23, 2016):

Usually something like https://github.com/Squirrel/Squirrel.Windows/pull/803/files#diff-dc4101658eac6c569618cb80ac0bffccR99. 7-Zip seems to have no problem opening the file

@anaisbetts commented on GitHub (Aug 23, 2016): Usually something like https://github.com/Squirrel/Squirrel.Windows/pull/803/files#diff-dc4101658eac6c569618cb80ac0bffccR99. 7-Zip seems to have no problem opening the file
claunia commented 2026-01-29 22:06:56 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (Aug 23, 2016):

Cool. I'll try to setup a test Tuesday when I'm back in front of a machine.

@adamhathcock commented on GitHub (Aug 23, 2016): Cool. I'll try to setup a test Tuesday when I'm back in front of a machine.
claunia commented 2026-01-29 22:06:57 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (Sep 27, 2016):

Coming up with nothing. Then again, I don't think I know how to properly use the Packaging API.

@adamhathcock commented on GitHub (Sep 27, 2016): Coming up with nothing. Then again, I don't think I know how to properly use the Packaging API.
claunia commented 2026-01-29 22:06:57 +00:00
Author
Owner
Copy Link

@anaisbetts commented on GitHub (Sep 27, 2016):

@adamhathcock You and me both fam. An easier test if you're playing around with it is to use the NuGet library, the ZipPackage class there is super straightforward and hits this bug

@anaisbetts commented on GitHub (Sep 27, 2016): @adamhathcock You and me both fam. An easier test if you're playing around with it is to use the NuGet library, the `ZipPackage` class there is super straightforward and hits this bug
claunia commented 2026-01-29 22:06:57 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (Sep 28, 2016):

@paulcbetts I must be thick because I just downloaded the Squirrel.Core.1.1.0.0.nupkg and can enumerate the parts

 using (Stream stream = File.Open("Squirrel.Core.1.1.0.0.nupkg", FileMode.Open, FileAccess.ReadWrite))
            {
                var package = ZipPackage.Open(stream);
                foreach (var part in package.GetParts())
                {
                    Console.WriteLine(part.Uri);
                }
            }
@adamhathcock commented on GitHub (Sep 28, 2016): @paulcbetts I must be thick because I just downloaded the Squirrel.Core.1.1.0.0.nupkg and can enumerate the parts ``` using (Stream stream = File.Open("Squirrel.Core.1.1.0.0.nupkg", FileMode.Open, FileAccess.ReadWrite)) { var package = ZipPackage.Open(stream); foreach (var part in package.GetParts()) { Console.WriteLine(part.Uri); } } ```
claunia commented 2026-01-29 22:06:58 +00:00
Author
Owner
Copy Link

@anaisbetts commented on GitHub (Oct 21, 2016):

@adamhathcock Lemme try it again with the latest SharpCompress

@anaisbetts commented on GitHub (Oct 21, 2016): @adamhathcock Lemme try it again with the latest SharpCompress
claunia commented 2026-01-29 22:06:58 +00:00
Author
Owner
Copy Link

@damieng commented on GitHub (May 19, 2017):

So I've found what is causing this - it's the check to make sure the version requirement from the Central Directory File Header matches the version requirement of the ZIP Local File Header contained within it.

Unfortunately for SharpCompress generated files there is a mismatch.

ZIP central directory file header is set to 10 at https://github.com/adamhathcock/sharpcompress/blob/master/src/SharpCompress/Writers/Zip/ZipCentralDirectoryEntry.cs#L33

ZIP local file header is set to 20 at https://github.com/adamhathcock/sharpcompress/blob/master/src/SharpCompress/Writers/Zip/ZipWriter.cs#L162

I suspect we can probably just change the 20 to a 10 in the ZipWriter? I can send a PR with the fix and a test if this is the case. Alternatively I guess we could change the central one to 20 but that might reduce compatibility. I guess a lot of tools outside of Microsoft's Packaging are only using one of the two.

@damieng commented on GitHub (May 19, 2017): So I've found what is causing this - it's the check to make sure the version requirement from the Central Directory File Header matches the version requirement of the ZIP Local File Header contained within it. Unfortunately for SharpCompress generated files there is a mismatch. ZIP central directory file header is set to 10 at https://github.com/adamhathcock/sharpcompress/blob/master/src/SharpCompress/Writers/Zip/ZipCentralDirectoryEntry.cs#L33 ZIP local file header is set to 20 at https://github.com/adamhathcock/sharpcompress/blob/master/src/SharpCompress/Writers/Zip/ZipWriter.cs#L162 I suspect we can probably just change the 20 to a 10 in the ZipWriter? I can send a PR with the fix and a test if this is the case. Alternatively I guess we could change the central one to 20 but that might reduce compatibility. I guess a lot of tools outside of Microsoft's Packaging are only using one of the two.
claunia commented 2026-01-29 22:06:58 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (May 19, 2017):

That's definitely a problem.

Quickly reviewing APPNOTE.txt again I should be dynamically changing the version needed to extract based on features.

Changing to 10 is probably the benign thing to do for now. I'll investigate more by looking at other implementations next week.

@adamhathcock commented on GitHub (May 19, 2017): That's definitely a problem. Quickly reviewing APPNOTE.txt again I should be dynamically changing the version needed to extract based on features. Changing to 10 is probably the benign thing to do for now. I'll investigate more by looking at other implementations next week.
claunia commented 2026-01-29 22:06:58 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (May 19, 2017):

Maybe it's just the fact that it's mismatched is the problem. Otherwise I would have said 20 is the proper thing for now.

@adamhathcock commented on GitHub (May 19, 2017): Maybe it's just the fact that it's mismatched is the problem. Otherwise I would have said 20 is the proper thing for now.
claunia commented 2026-01-29 22:06:59 +00:00
Author
Owner
Copy Link

@damieng commented on GitHub (May 19, 2017):

It looks like Deflate is only supported in version '20' so I think you're right - changing the ZIP central dictory file header to '20' would be the smallest fix.

If there was a need to support '10' then it would need to also not use deflate or encryption.

@damieng commented on GitHub (May 19, 2017): It looks like Deflate is only supported in version '20' so I think you're right - changing the ZIP central dictory file header to '20' would be the smallest fix. If there was a need to support '10' then it would need to also not use deflate or encryption.
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
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#122
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?