Skip to content

Conversation

@grendello
Copy link
Contributor

@grendello grendello commented Jan 25, 2022

Context: dbe3236

dbe3236 fixed an issue of data corruption when deleting files from the
archive by properly creating temporary streams when libzip requested
data modifications. However, a bug crept in which causes the temporary
stream to be rewound, instead of the destination one, when data is
written back. This has the effect of overwriting the ZIP local file
header structure with, essentially, random data and causes errors
similar to:

error XABBA7000: Xamarin.Tools.Zip.ZipException: Zip archive inconsistent: entry 0: invalid WinZip AES extra field
Xamarin.Android.Common.targets(2289,2): error ANDZA0000: zip W 01-25 15:03:13  9516 56652 WARNING: header mismatch

The issue can be seen by either attempting to unpack the archive:

$ 7z x base.zip
ERROR: Headers Error : dex/classes.dex
ERROR: Headers Error : root/assemblies/assemblies.blob

$ unzip base.zip
file #9:  bad zipfile offset (local header sig):  49783
file #10:  bad zipfile offset (local header sig):  127988

Or by looking at the file with zipinfo -vv base.zip:

Central directory entry #10:
---------------------------

  There are an extra 9 bytes preceding this file.

  root/assemblies/assemblies.blob

  offset of local header from start of archive:   127988 (000000000001F3F4h) bytes

Not only the local header is thought to be at the incorrect offset, it
also doesn't have the correct signature:

# show valid header
$ hexdump -n 4 -C base.zip
00000000  50 4b 03 04                                       |PK..|

# show invalid header
$ hexdump -s 127988 -n 4 -C base.zip
0001f3f4  49 be 37 16                                       |I.7.|

$ hexdump -s $((127988 - 9)) -n 4 -C base.zip
0001f3eb  88 26 de 2c                                       |.&.,|

Attempt to fix the problem by rewinding the destination stream on write.

Fix found by Dean Ellis (@dellis1972)

@grendello grendello requested a review from dellis1972 January 25, 2022 17:12
Context: dbe3236

dbe3236 fixed an issue of data corruption when deleting files from the
archive by properly creating temporary streams when `libzip` requested
data modifications.  However, a bug crept in which causes the temporary
stream to be rewound, instead of the destination one, when data is
written back.  This has the effect of overwriting the ZIP local file
header structure with, essentially, random data and causes errors
similar to:

    error XABBA7000: Xamarin.Tools.Zip.ZipException: Zip archive inconsistent: entry 0: invalid WinZip AES extra field
    Xamarin.Android.Common.targets(2289,2): error ANDZA0000: zip W 01-25 15:03:13  9516 56652 WARNING: header mismatch

The issue can be seen by either attempting to unpack the archive:

    $ 7z x base.zip
    ERROR: Headers Error : dex/classes.dex
    ERROR: Headers Error : root/assemblies/assemblies.blob

    $ unzip base.zip
    file #9:  bad zipfile offset (local header sig):  49783
    file #10:  bad zipfile offset (local header sig):  127988

Or by looking at the file with `zipinfo -vv base.zip`:

    Central directory entry #10:
    ---------------------------

      There are an extra 9 bytes preceding this file.

      root/assemblies/assemblies.blob

      offset of local header from start of archive:   127988 (000000000001F3F4h) bytes

Not only the local header is thought to be at the incorrect offset, it
also doesn't have the correct signature:

    # show valid header
    $ hexdump -n 4 -C base.zip
    00000000  50 4b 03 04                                       |PK..|

    # show invalid header
    $ hexdump -s 127988 -n 4 -C base.zip
    0001f3f4  49 be 37 16                                       |I.7.|

    $ hexdump -s $((127988 - 9)) -n 4 -C base.zip
    0001f3eb  88 26 de 2c                                       |.&.,|

Attempt to fix the problem by rewinding the destination stream on write.

Fix found by Dean Ellis (@dellis1972)
@grendello grendello merged commit 9bccd74 into main Jan 25, 2022
@grendello grendello deleted the fix-stream-rewind branch January 25, 2022 18:54
grendello pushed a commit to dotnet/android-tools that referenced this pull request Jan 26, 2022
Bump LibZipSharp to 2.0.3

Context: dotnet/android-libzipsharp#104

Changes

- Don't rewind the temporary stream on write to the main one

dotnet/android-libzipsharp@2.0.2...2.0.3
jonpryor pushed a commit to dotnet/android that referenced this pull request Jan 28, 2022
Context: dotnet/android-libzipsharp#104

Changes: dotnet/android-libzipsharp@2.0.2...2.0.3

  * dotnet/android-libzipsharp@9bccd74: Don't rewind the temporary stream on write to the main one (#104)

We are seeing the following error on some builds

	error XABBA7000: Xamarin.Tools.Zip.ZipException: Zip archive inconsistent: entry 0: invalid WinZip AES extra field

Verison 2.0.2 of LibZipSharp was supposed to fix this, but it had a
bug (dotnet/android-libzipsharp#104), which would cause corruption in the
zip file itself.

We believe that this issue has been fixed in LibZipSharp 2.0.3; Bump!
@grendello grendello restored the fix-stream-rewind branch August 23, 2022 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants