From 78558d5e10a30c88c0b564e69963a0f0188c1dbd Mon Sep 17 00:00:00 2001
From: Ian Lance Taylor <iant@golang.org>
Date: Wed, 11 Jan 2023 12:17:20 -0800
Subject: [PATCH] archive/zip: use base offset 0 if it has a valid entry

In CL 408734 we introduced a fall back to base offset 0 if reading a
directory entry at the computed base offset failed. We have now found
a file in the wild for which the computed base offset is incorrect,
but happens to refer to a valid directory entry. In this CL, we change
the fallback such that if the first directory header relative to base
offset 0 is valid, we just use base offset 0.

Change-Id: Ia9ace20c1065d1f651035f16f7d91d741ab1dbf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/461598
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
---
 src/archive/zip/reader.go                 |  28 +++++++++++-----------
 src/archive/zip/reader_test.go            |  18 ++++++++++++++
 src/archive/zip/testdata/test-badbase.zip | Bin 0 -> 1170 bytes
 3 files changed, 32 insertions(+), 14 deletions(-)
 create mode 100644 src/archive/zip/testdata/test-badbase.zip

diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index c29837836b..3e96d0ecc9 100644
--- a/src/archive/zip/reader.go
+++ b/src/archive/zip/reader.go
@@ -151,20 +151,6 @@ func (z *Reader) init(r io.ReaderAt, size int64) error {
 	for {
 		f := &File{zip: z, zipr: r}
 		err = readDirectoryHeader(f, buf)
-
-		// For compatibility with other zip programs,
-		// if we have a non-zero base offset and can't read
-		// the first directory header, try again with a zero
-		// base offset.
-		if err == ErrFormat && z.baseOffset != 0 && len(z.File) == 0 {
-			z.baseOffset = 0
-			if _, err = rs.Seek(int64(end.directoryOffset), io.SeekStart); err != nil {
-				return err
-			}
-			buf.Reset(rs)
-			continue
-		}
-
 		if err == ErrFormat || err == io.ErrUnexpectedEOF {
 			break
 		}
@@ -627,6 +613,20 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, baseOffset
 	if o := baseOffset + int64(d.directoryOffset); o < 0 || o >= size {
 		return nil, 0, ErrFormat
 	}
+
+	// If the directory end data tells us to use a non-zero baseOffset,
+	// but we would find a valid directory entry if we assume that the
+	// baseOffset is 0, then just use a baseOffset of 0.
+	// We've seen files in which the directory end data gives us
+	// an incorrect baseOffset.
+	if baseOffset > 0 {
+		off := int64(d.directoryOffset)
+		rs := io.NewSectionReader(r, off, size-off)
+		if readDirectoryHeader(&File{}, rs) == nil {
+			baseOffset = 0
+		}
+	}
+
 	return d, baseOffset, nil
 }
 
diff --git a/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go
index 1594b2648e..70ad260cc5 100644
--- a/src/archive/zip/reader_test.go
+++ b/src/archive/zip/reader_test.go
@@ -126,6 +126,24 @@ var tests = []ZipTest{
 			},
 		},
 	},
+	{
+		Name:    "test-badbase.zip",
+		Comment: "This is a zipfile comment.",
+		File: []ZipTestFile{
+			{
+				Name:     "test.txt",
+				Content:  []byte("This is a test text file.\n"),
+				Modified: time.Date(2010, 9, 5, 12, 12, 1, 0, timeZone(+10*time.Hour)),
+				Mode:     0644,
+			},
+			{
+				Name:     "gophercolor16x16.png",
+				File:     "gophercolor16x16.png",
+				Modified: time.Date(2010, 9, 5, 15, 52, 58, 0, timeZone(+10*time.Hour)),
+				Mode:     0644,
+			},
+		},
+	},
 	{
 		Name:   "r.zip",
 		Source: returnRecursiveZip,
diff --git a/src/archive/zip/testdata/test-badbase.zip b/src/archive/zip/testdata/test-badbase.zip
new file mode 100644
index 0000000000000000000000000000000000000000..245a62cb6d116672b6c2e3201553cd0e047e0218
GIT binary patch
literal 1170
zcmWIWW@Zs#U|`^2XiQYKJ#hW)VM!oQ3M?YSP?B0)qE}K;5*otEz+CvJ$)^m6ODnh;
z7+JnDGBAL3a-Te*6UMN}rFGJkM?wmLfr}<gn>l&aaaM)^pwV1FgBTd*)~VY5GrSri
z$jrb1!XgYZ4C(m=8L36d`8oMThGrFpW_ksA>0oQD44Qqcff&u2&Hz7mUM?w+fxMm`
zE<r$A0EqcH*ns5QVt;)g#ggvm>&U=x?Zy@V2qPe0vcxr_Bsf2<q&%@GmBBG3KPgqg
zGdD3kH7GSPrLyp3stp4J(_&8-$B>F!C6nVlf&(QE?5{rm?pSGb*exy9+#I&RurKk-
z+m)0yRCPG=d-HHDZeo&8l5*8w*j(kQ)h25CKV`;*MHBS|=I94cI>i2RTbstM*llm)
zO8IWizic_XckQR<_w%av+wcEed*5>UK?&z&H{O<QxS2Ek{9LOl<@FaLM5a7Ry?<nS
zs%pE)oUrhRWxMyz{MdD-f7-OLFT3`heVDOgZ;FZ3gB?$$?&NQ9&YUQ=WCF|QoPTOU
z(^wNSSWQh$1>(96e0boH{+WCE(bu95f2^3EZ4O=VNZ?3)UvtyJg5QDRcf$^EI~Jki
zWtQzJsL_x*GnsW)scd<ESYjks;`tZqHBT<Y)hx<Uue%)5uO4%LL;i_Q{U|@K*PqQc
zq;0mG>27+nNv&O9`(nP~TLZC!C)&dpmb9Gex+~DL%5m{kC!2{41u2r}!o#L;7=ONQ
zE-qq{y7hnokNZ9E^>G%L>tA=N)+ZKnN{AiY63=$`mQ27(iv*7buak|7iv?tIZF@fl
zt>|OAAsR5{#f{tFon;*@eKst4U%}L6{!DE$Q;-Gc;z`Q`4>jq0{kyW^?3@=d0sB55
zxoLcI^%PC5#tA<g%Dq?;-I_9V*mnJidBycd+`*;l>eEd>m$^D<JncI6XhVLr;fyj-
zwZq{LUR`An=sq6%?&3$)sH&;}9dG$e1>R^;2N$vDP24)a*LyE?_;6T$0ZaWgUlR!)
zO)1Y`?`E^OggslIl9kNpY|u0@YV$J_L(}Dqa{cO(OH|YIR<91PW~;8;CG9SvxT%P1
zf&1?rTi;(l9{BL-LFV7-o3%9b_5H&Yx{{nGExEP)?O%T1_U}*5o%1=o!$7pc+~7{P
zVEy`CyQ(f_ZB6^{)1;6Rw%g;&&eO(;vcI48f3W$#VE69d#v5C|RN3w=iCp_vEP-JK
z2X8m?^Q2e6G|k}Y>gTe~DWNIAn~_P58CTww04Zev=23<vjUW~xC$mCwGFtY6n1(C6
zLQDghv!pQ^NMg^tK$Ah47sF&$P!?ri0>UUDy$e_XNHK(DWELv`QKCXsW<gqJPO3t3
Ler|4RUWpz6DXh9)

literal 0
HcmV?d00001

-- 
2.50.0