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

archive/zip: error reading large files from zip64 files #7069

Closed
bradfitz opened this issue Jan 6, 2014 · 6 comments
Closed

archive/zip: error reading large files from zip64 files #7069

bradfitz opened this issue Jan 6, 2014 · 6 comments
Assignees
Milestone

Comments

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 6, 2014

I received a private bug report from a user who doesn't want to create an account to
file a bug here.

Unfortunately, the bug report has no repro case.

In summary: in readDirectoryHeader, we can read too far into a zip64 Extra block.  The
Extra block is a repeated list of (extra tag + size + extra bytes of declared size). 
When we see the extra tag of 1 (zip64ExtraId), we then parse the extra block assuming
the buffer is everything remaining in the file, instead of capping it at the declared
size.  That matters, because the Extra field is a variably-sized structure:

        Value      Size       Description
        -----      ----       -----------
(ZIP64) 0x0001     2 bytes    Tag for this "extra" block type
        Size       2 bytes    Size of this "extra" block
        Original 
        Size       8 bytes    Original uncompressed file size
        Compressed
        Size       8 bytes    Size of compressed data
        Relative Header
        Offset     8 bytes    Offset of local header record
        Disk Start
        Number     4 bytes    Number of the disk on which
                              this file starts 

      This entry in the Local header MUST include BOTH original
      and compressed file size fields. If encrypting the 
      central directory and bit 13 of the general purpose bit
      flag is set indicating masking, the value stored in the
      Local Header for the original file size will be zero.
@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Jan 6, 2014

Comment 1:

https://golang.org/cl/48150043

Status changed to Started.

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Jan 6, 2014

Comment 2:

Labels changed: added release-go1.2.1.

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Jan 6, 2014

Comment 3:

This issue was closed by revision 02a15e7.

Status changed to Fixed.

@jktomer
Copy link

@jktomer jktomer commented Jan 7, 2014

Comment 4:

Here's a suitable replacement for archive/zip/testdata/zip64.zip that causes the
archive/zip tests to fail before fe0a21b4dd1d but not after.
By the way, I have anecdotal evidence that there exist zips with only an 8-bit zip64
header (i.e. one with the uncompressed file size but not the compressed file size or
file offset), contrary to the note in the file-format description quoted above. However
I can't share the instance I have, and haven't been able to create another one with this
property. The fix that's in place now will still work, so this is just an academic point
for posterity.

Attachments:

  1. zip64.zip (266 bytes)
@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Jan 8, 2014

Comment 5:

This issue was updated by revision e7c2170.

R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/49180043
@rsc
Copy link
Contributor

@rsc rsc commented Feb 16, 2014

Comment 6:

Go 1.2.1 is for critical bugs; this one is not.
Anyone affected by this can copy the new archive/zip code into a separate package.

Labels changed: added release-go1.3, removed release-go1.2.1.

@bradfitz bradfitz added fixed labels Feb 16, 2014
@bradfitz bradfitz self-assigned this Feb 16, 2014
@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.