archive/zip: Reader.Read returns ErrChecksum on certain files

[ericpromislow]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.7.1 darwin/amd64

Does this issue reproduce with the latest release?

Don't know

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/pivotal/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.7.1/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.7.1/libexec/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/60/czc9bh9j6w17gl13v_g9vxs80000gn/T/go-build433256889=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

This is fun.

1. Create a zip file on Windows, or any other platform where the
   directory entries don't have the 'd' bit set. Here's the one we worked on: https://github.com/cloudfoundry/cf-acceptance-tests/blob/master/assets/java-spring/java-spring.jar

2. Copy bad-win.zip to orig-bad-win.zip, so you can run step 3 repeatedly:

3. Run the zip file through some ruby code like this:

   require 'zip'
   def get_dirs_from_zip(zip_path)
   Zip::File.open(zip_path) do |in_zip|
   in_zip.select(&:directory?)
   end
   end

   dir_list = get_dirs_from_zip(zip_path)
   Zip::File.open(zip_file) do |in_zip|
   dir_list.each do |dir|
   in_zip.remove(dir)
   end
   end

4. The go code is attached in a file called transform.go

5. I ran this command (note that the code wants ok-win.tar to exist in advance):

touch ok-win.tar ; go run transform.go bad-win.zip ok-win.tar

What did you expect to see?

I expected to see a tar file called ok-win.tar

What did you see instead?

"zip: checksum error"

Notes

There are three likely sources for this error (4 but we're allowing winzip to not set the 'd' bit because zip is weird about directories anyway):

1. Our ruby code
2. The ruby zip library
3. The go archive/zip library

Our ruby code seems ok, because it's not a problem with the zip files where the 'd' bit is set.

The error might be in the ruby zip library, but I can't really report it until I have a better idea
of the cause of the checksum error. I left some printf's in the code, so you can see the error occurs in the call io.Copy(tarWriter, zipFileReader), and not earlier calls that read the zip file.

[ericpromislow]

transform.go.txt

[marcellanz]

I've investigated this issue and extracted both the go and ruby code together with a test to reproduce the issue and a possible fix for the issuers ruby code here:
https://github.com/marcellanz/go-21876

what happens

IMO, this is a non-issue from Go's point of view, here is why:

I was able to reproduce the issue on "go version go1.11.5 darwin/amd64" with the *.jar file and ruby code provided to this issue.

Also, I was able to create a similar zip file using ZIP(1L) on macOS that behaved similarly with the ruby script provided and then processed with the go code of this issue.

What seems to be happening is the following:

1. The given *.jar file contains data descriptor entries [4] for each entry in the zip file.

2. The produced zip file by the ruby code [2] left bit 3 set on 1 of the general purpose bit flag that is part of the central directory header and that indicates the presence of a corresponding data descriptor entry for a file within the zip file…

3. … but Ruby's zip implementation Rubyzip [5] does not write data descriptor entries at all [6] (at least for all non-encryption zip files IMO / see ::Zip::NullDecrypter) and ignores the general purpose bit flag on bit 3 that was copied over initially [7].

4. Go's zip reader implementation reads the general purpose flag on bit 3 when reading the central directory header entry of every file in the zip file [8] and then expects to get to find a data descriptor entry after the corresponding file data section within the zip file.

5. Ultimately Go's zip reader reads a wrong CRC [9] since no data descriptor entry is present and then emits ErrChecksum

ZIP file structure (simplified)

      [local file header 1]
      [file data         1]
      [data descriptor   1] <- these are missing (incl. crc) after Rubyzip modified the zip file
      ...
      [local file header n]
      [file data         n]
      [data descriptor   n]
      ...
      [central directory header 1] <- bit 3 on the general purpose flags is still set
      ...
      [central directory header n]

fix

To fix this issue on the issuers side, general purpose flags can be cleared.

e.gp_flags &= ~0x8

Also (I'm not a ruby expert) Rubyzip's zip output stream might have to be fixed for entries that where copied over from an existing zip file. There is an issue [10] on Rubyzip that is Open and indicates to be similar to Issue21876.

Similar to Go's zip reader, macOS's Archive Utility.app cannot open the ZIP file produced by Rubyzip.

issue reproduced

Here we produce a zip file with data descriptors included for zip file entries using "-fd" flag with ZIP(1L) on macOS. Then, as described in the issue, the ruby script is used to remove directories out of the zip file and the zip file is transformed to a tar file using Go.

Run the following script (found here: https://github.com/marcellanz/go-21876) to reproduce the issue:

cd src/org.golang.go.issues/21876
# see it fail
./run.sh
# see it not fail with cleared gp_flags (see below)
./run.sh -fixed

[dsnet]

Hi @marcellanz, thank you for your wonderful analysis. It's thorough and seems to conclusively show that the issue is with buggy file generation in Ruby. Given that there is nothing actionable on Go's end, I'm closing the issue.