adamhathcock / sharpcompress

SharpCompress is a fully managed C# library to deal with many compression types and formats.
MIT License
2.24k stars 480 forks source link

Exception extracting APK archives #54

Open tritao opened 9 years ago

tritao commented 9 years ago

It has come to my attention that certain Android package archives (APKs) are not extractable by SharpCompress due to the following exception:

Unhandled Exception:
System.ArgumentException: Destination array is not long enough to copy all the items in the collection. Check array index and length.
  at System.ThrowHelper.ThrowArgumentException (ExceptionResource resource) [0x00000] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/external/referencesource/mscorlib/system/throwhelper.cs:74 
  at System.BitConverter.ToUInt16 (System.Byte[] value, Int32 startIndex) [0x0002c] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/external/referencesource/mscorlib/system/bitconverter.cs:293 
  at SharpCompress.Common.Zip.Headers.ZipFileEntry.LoadExtra (System.Byte[] extra) [0x00007] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/Headers/ZipFileEntry..cs:71 
  at SharpCompress.Common.Zip.Headers.LocalEntryHeader.Read (System.IO.BinaryReader reader) [0x0008b] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/Headers/LocalEntryHeader.cs:39 
  at SharpCompress.Common.Zip.ZipHeaderFactory.ReadHeader (UInt32 headerBytes, System.IO.BinaryReader reader) [0x00063] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/ZipHeaderFactory.cs:42 
  at SharpCompress.Common.Zip.SeekableZipHeaderFactory.GetLocalHeader (System.IO.Stream stream, SharpCompress.Common.Zip.Headers.DirectoryEntryHeader directoryEntryHeader) [0x0001d] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/SeekableZipHeaderFactory.cs:69 
  at SharpCompress.Common.Zip.SeekableZipFilePart.LoadLocalHeader () [0x0000c] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/SeekableZipFilePart.cs:35 
  at SharpCompress.Common.Zip.SeekableZipFilePart.GetCompressedStream () [0x0000b] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/SeekableZipFilePart.cs:21 
  at SharpCompress.Archive.Zip.ZipArchiveEntry.OpenEntryStream () [0x00000] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Archive/Zip/ZipArchiveEntry.cs:17 
  at System.IO.Compression.ZipArchiveEntry.Open () [0x0007f] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/ZipArchiveEntry.cs:115 
  at System.IO.Compression.ZipFileExtensions.ExtractToFile (System.IO.Compression.ZipArchiveEntry source, System.String destinationFileName, Boolean overwrite) [0x00039] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression.FileSystem/ZipFileExtensions.cs:115 
  at System.IO.Compression.ZipFileExtensions.ExtractToDirectory (System.IO.Compression.ZipArchive source, System.String destinationDirectoryName) [0x00095] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression.FileSystem/ZipFileExtensions.cs:91 
  at System.IO.Compression.ZipFile.ExtractToDirectory (System.String sourceArchiveFileName, System.String destinationDirectoryName, System.Text.Encoding entryNameEncoding) [0x0001a] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression.FileSystem/ZipFile.cs:131 
  at System.IO.Compression.ZipFile.ExtractToDirectory (System.String sourceArchiveFileName, System.String destinationDirectoryName) [0x00000] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression.FileSystem/ZipFile.cs:117 
  at ExtractTest.MainClass.Main (System.String[] args) [0x00024] in /Users/pjcollins/Desktop/Repro/Program.cs:12 
[ERROR] FATAL UNHANDLED EXCEPTION: System.ArgumentException: Destination array is not long enough to copy all the items in the collection. Check array index and length.
  at System.ThrowHelper.ThrowArgumentException (ExceptionResource resource) [0x00000] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/external/referencesource/mscorlib/system/throwhelper.cs:74 
  at System.BitConverter.ToUInt16 (System.Byte[] value, Int32 startIndex) [0x0002c] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/external/referencesource/mscorlib/system/bitconverter.cs:293 
  at SharpCompress.Common.Zip.Headers.ZipFileEntry.LoadExtra (System.Byte[] extra) [0x00007] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/Headers/ZipFileEntry..cs:71 
  at SharpCompress.Common.Zip.Headers.LocalEntryHeader.Read (System.IO.BinaryReader reader) [0x0008b] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/Headers/LocalEntryHeader.cs:39 
  at SharpCompress.Common.Zip.ZipHeaderFactory.ReadHeader (UInt32 headerBytes, System.IO.BinaryReader reader) [0x00063] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/ZipHeaderFactory.cs:42 
  at SharpCompress.Common.Zip.SeekableZipHeaderFactory.GetLocalHeader (System.IO.Stream stream, SharpCompress.Common.Zip.Headers.DirectoryEntryHeader directoryEntryHeader) [0x0001d] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/SeekableZipHeaderFactory.cs:69 
  at SharpCompress.Common.Zip.SeekableZipFilePart.LoadLocalHeader () [0x0000c] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/SeekableZipFilePart.cs:35 
  at SharpCompress.Common.Zip.SeekableZipFilePart.GetCompressedStream () [0x0000b] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/SeekableZipFilePart.cs:21 
  at SharpCompress.Archive.Zip.ZipArchiveEntry.OpenEntryStream () [0x00000] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/SharpCompress/Archive/Zip/ZipArchiveEntry.cs:17 
  at System.IO.Compression.ZipArchiveEntry.Open () [0x0007f] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression/ZipArchiveEntry.cs:115 
  at System.IO.Compression.ZipFileExtensions.ExtractToFile (System.IO.Compression.ZipArchiveEntry source, System.String destinationFileName, Boolean overwrite) [0x00039] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression.FileSystem/ZipFileExtensions.cs:115 
  at System.IO.Compression.ZipFileExtensions.ExtractToDirectory (System.IO.Compression.ZipArchive source, System.String destinationDirectoryName) [0x00095] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression.FileSystem/ZipFileExtensions.cs:91 
  at System.IO.Compression.ZipFile.ExtractToDirectory (System.String sourceArchiveFileName, System.String destinationDirectoryName, System.Text.Encoding entryNameEncoding) [0x0001a] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression.FileSystem/ZipFile.cs:131 
  at System.IO.Compression.ZipFile.ExtractToDirectory (System.String sourceArchiveFileName, System.String destinationDirectoryName) [0x00000] in /private/tmp/source-mono-mac-4.0.0-branch/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.0/mcs/class/System.IO.Compression.FileSystem/ZipFile.cs:117 
  at ExtractTest.MainClass.Main (System.String[] args) [0x00024] in /Users/pjcollins/Desktop/Repro/Program.cs:12 

From a simple reading of some Zip specs it seems that extra data always should be in a multiple of 2 bytes so I did this simple fix which gets these archives to work:

--- a/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/Headers/ZipFileEntry..cs
+++ b/mcs/class/System.IO.Compression/SharpCompress/Common/Zip/Headers/ZipFileEntry..cs
@@ -66,6 +66,9 @@ namespace SharpCompress.Common.Zip.Headers

         protected void LoadExtra(byte[] extra)
         {
+            if (extra.Length % 2 != 0)
+                return;
+                       
             for (int i = 0; i < extra.Length;)
             {
                 ExtraDataType type = (ExtraDataType) BitConverter.ToUInt16(extra, i);
tritao commented 9 years ago

I was just thinking about this issue again and while this change fixed my original problem I'm not sure it's correct... we read the length as part of the extra byte array so nothing actually guarantees that it needs to have a multiple-of-2 length. This could actually lead to not correctly loading the extra data for some archives.

Maybe it would be best to fix by checking that we have at least 2 more bytes before trying to convert the data with BitConverter. What do you think?

adamhathcock commented 9 years ago

Been so long since I've looked at this, trying to wrap my head around it again...

glancing at the spec again, it's supposed to be pairs of 2 byte headers for extended values. Ensuring that there are actually 2 bytes to read before trying to read them seems to make the most sense.

Just surprised by how many things write bad zip headers.