archive/zip: fix reader-side Zip64 edge cases

The 0xffff directorySize was a typo, and File.zip64 was unused.

The ErrFormat was intentional, but it's actually possible to have a
valid zip file with compressed size 2³²-1, for example by storing
uncompressed a file of size 2³²-1 using Info-ZIP. The following CL
introduces a couple such files (infozip-store-4g-minus-1 and
infozip-offset-eq-4g).

Fixes #31692
Fixes #56249
Updates #14185
Updates #13367
Updates #13166

Change-Id: I503805cace50316a665633d43dcc7fa46a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/779180
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
Filippo Valsorda 2026-05-18 13:49:24 +02:00 committed by Gopher Robot
parent 8b672822b2
commit 8a69bfb1bb
2 changed files with 24 additions and 34 deletions

View file

@ -63,7 +63,6 @@ type File struct {
zip *Reader
zipr io.ReaderAt
headerOffset int64 // includes overall ZIP archive baseOffset
zip64 bool // zip64 extended information extra field presence
}
// OpenReader will open the Zip file specified by name and return a ReadCloser.
@ -406,10 +405,6 @@ func readDirectoryHeader(f *File, r io.Reader) error {
f.NonUTF8 = f.Flags&0x800 == 0
}
needUSize := f.UncompressedSize == ^uint32(0)
needCSize := f.CompressedSize == ^uint32(0)
needHeaderOffset := f.headerOffset == int64(^uint32(0))
// Best effort to find what we need.
// Other zip authors might not even follow the basic format,
// and we'll just ignore the Extra content in that case.
@ -425,28 +420,23 @@ parseExtras:
switch fieldTag {
case zip64ExtraID:
f.zip64 = true
// update directory values from the zip64 extra block.
// They should only be consulted if the sizes read earlier
// are maxed out.
// See golang.org/issue/13367.
if needUSize {
needUSize = false
// See go.dev/issue/13367 and go.dev/issue/31692.
if f.UncompressedSize == ^uint32(0) {
if len(fieldBuf) < 8 {
return ErrFormat
}
f.UncompressedSize64 = fieldBuf.uint64()
}
if needCSize {
needCSize = false
if f.CompressedSize == ^uint32(0) {
if len(fieldBuf) < 8 {
return ErrFormat
}
f.CompressedSize64 = fieldBuf.uint64()
}
if needHeaderOffset {
needHeaderOffset = false
if f.headerOffset == int64(^uint32(0)) {
if len(fieldBuf) < 8 {
return ErrFormat
}
@ -509,20 +499,6 @@ parseExtras:
}
}
// Assume that uncompressed size 2³²-1 could plausibly happen in
// an old zip32 file that was sharding inputs into the largest chunks
// possible (or is just malicious; search the web for 42.zip).
// If needUSize is true still, it means we didn't see a zip64 extension.
// As long as the compressed size is not also 2³²-1 (implausible)
// and the header is not also 2³²-1 (equally implausible),
// accept the uncompressed size 2³²-1 as valid.
// If nothing else, this keeps archive/zip working with 42.zip.
_ = needUSize
if needCSize || needHeaderOffset {
return ErrFormat
}
return nil
}
@ -605,7 +581,7 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, baseOffset
d.comment = string(b[:l])
// These values mean that the file can be a zip64 file
if d.directoryRecords == 0xffff || d.directorySize == 0xffff || d.directoryOffset == 0xffffffff {
if d.directoryRecords == 0xffff || d.directorySize == 0xffffffff || d.directoryOffset == 0xffffffff {
p, err := findDirectory64End(r, directoryEndOffset)
if err == nil && p >= 0 {
directoryEndOffset = p

View file

@ -14,7 +14,6 @@ import (
"hash"
"internal/testenv"
"io"
"runtime"
"slices"
"strings"
"testing"
@ -494,8 +493,8 @@ func suffixIsZip64(t *testing.T, zip sizedReaderAt) bool {
// Zip64 is required if the total size of the records is uint32max.
func TestZip64LargeDirectory(t *testing.T) {
if runtime.GOARCH == "wasm" {
t.Skip("too slow on wasm")
if testenv.CPUIsSlow() {
t.Skip("too slow")
}
if testing.Short() {
t.Skip("skipping in short mode")
@ -538,15 +537,30 @@ func TestZip64LargeDirectory(t *testing.T) {
}
t.Run("uint32max-1_NoZip64", func(t *testing.T) {
t.Parallel()
if generatesZip64(t, gen(uint32max-1)) {
buf := new(rleBuffer)
w := NewWriter(buf)
gen(uint32max - 1)(w)
if suffixIsZip64(t, buf) {
t.Error("unexpected zip64")
}
if _, err := NewReader(buf, buf.Size()); err != nil {
t.Errorf("NewReader: %v", err)
}
})
t.Run("uint32max_HasZip64", func(t *testing.T) {
t.Parallel()
if !generatesZip64(t, gen(uint32max)) {
buf := new(rleBuffer)
w := NewWriter(buf)
gen(uint32max)(w)
if !suffixIsZip64(t, buf) {
t.Error("expected zip64")
}
// Round-trip through NewReader. With CD size exactly 0xFFFFFFFF,
// records well below 0xFFFF, and dirOffset == 0, the only EOCD
// field that holds the placeholder is directorySize.
if _, err := NewReader(buf, buf.Size()); err != nil {
t.Errorf("NewReader: %v", err)
}
})
}