From b8562dda13f34b34facb68abb3b65ae94ef71cb0 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Wed, 5 Jun 2024 15:15:20 +0200 Subject: [PATCH 1/4] Sync upstream changes Notably includes https://github.com/golang/go/issues/66869 and `RemoveInsecurePaths()` for https://github.com/golang/go/issues/55356 Updates CI. --- .github/workflows/go.yml | 2 +- .github/workflows/vulncheck.yml | 2 +- file.go | 22 +++++++++++++++++++++ go.mod | 8 +++++--- go.sum | 33 ++++++-------------------------- reader.go | 13 +++++++++++-- testdata/comment-truncated.zip | Bin 0 -> 216 bytes testdata/readme.notzip | Bin 1906 -> 1906 bytes testdata/test-badbase.zip | Bin 0 -> 1170 bytes zipindex_test.go | 3 +++ 10 files changed, 49 insertions(+), 34 deletions(-) create mode 100644 testdata/comment-truncated.zip create mode 100644 testdata/test-badbase.zip diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index f2c3c39..8cea6b2 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -14,7 +14,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - go-version: [1.16.x,1.17.x,1.18.x] + go-version: [1.20.x,1.21.x,1.22.x] os: [ubuntu-latest] steps: - name: Set up Go ${{ matrix.go-version }} on ${{ matrix.os }} diff --git a/.github/workflows/vulncheck.yml b/.github/workflows/vulncheck.yml index 7b8dfe8..ea7d308 100644 --- a/.github/workflows/vulncheck.yml +++ b/.github/workflows/vulncheck.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go-version: [ 1.19 ] + go-version: [ 1.22.x ] steps: - name: Check out code into the Go module directory uses: actions/checkout@v3 diff --git a/file.go b/file.go index 0dee9be..79d1d05 100644 --- a/file.go +++ b/file.go @@ -22,7 +22,9 @@ import ( "fmt" "hash/crc32" "io" + "path/filepath" "sort" + "strings" "github.com/klauspost/compress/zstd" "github.com/tinylib/msgp/msgp" @@ -31,6 +33,7 @@ import ( //go:generate msgp -file $GOFILE -unexported // File is a sparse representation of a File inside a zip file. +// //msgp:tuple File type File struct { Name string // Name of the file as stored in the zip. @@ -77,6 +80,7 @@ func (f *File) OpenRaw(r io.Reader) (io.Reader, error) { } // Files is a collection of files. +// //msgp:ignore Files type Files []File @@ -174,6 +178,24 @@ func (f Files) Serialize() ([]byte, error) { return zstdEnc.EncodeAll(payload, res), nil } +// RemoveInsecurePaths will remove any file with path deemed insecure. +// This is files that fail either !filepath.IsLocal(file.Name) or contain a backslash. +func (f *Files) RemoveInsecurePaths() { + files := *f + for i, file := range files { + if file.Name == "" { + // Zip permits an empty file name field. + continue + } + // The zip specification states that names must use forward slashes, + // so consider any backslashes in the name insecure. + if !filepath.IsLocal(file.Name) || strings.Contains(file.Name, `\`) { + files = append(files[:i], files[:i+1]...) + } + } + *f = files +} + // Sort files by offset in zip file. // Typically, directories are already sorted by offset. // This will usually provide the smallest possible serialized size. diff --git a/go.mod b/go.mod index 5719244..5d1da60 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,10 @@ module github.com/minio/zipindex -go 1.16 +go 1.20 require ( - github.com/klauspost/compress v1.15.9 - github.com/tinylib/msgp v1.1.6 + github.com/klauspost/compress v1.17.8 + github.com/tinylib/msgp v1.1.9 ) + +require github.com/philhofer/fwd v1.1.2 // indirect diff --git a/go.sum b/go.sum index a4a2c86..bddbfed 100644 --- a/go.sum +++ b/go.sum @@ -1,27 +1,6 @@ -github.com/klauspost/compress v1.15.9 h1:wKRjX6JRtDdrE9qwa4b/Cip7ACOshUI4smpCQanqjSY= -github.com/klauspost/compress v1.15.9/go.mod h1:PhcZ0MbTNciWF3rruxRgKxI5NkcHHrHUDtV4Yw2GlzU= -github.com/philhofer/fwd v1.1.1 h1:GdGcTjf5RNAxwS4QLsiMzJYj5KEvPJD3Abr261yRQXQ= -github.com/philhofer/fwd v1.1.1/go.mod h1:gk3iGcWd9+svBvR0sR+KPcfE+RNWozjowpeBVG3ZVNU= -github.com/tinylib/msgp v1.1.6 h1:i+SbKraHhnrf9M5MYmvQhFnbLhAXSDWF8WWsuyRdocw= -github.com/tinylib/msgp v1.1.6/go.mod h1:75BAfg2hauQhs3qedfdDZmWAPcFMAvJE5b9rGOMufyw= -github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.0.0-20201022035929-9cf592e881e9/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +github.com/klauspost/compress v1.17.8 h1:YcnTYrq7MikUT7k0Yb5eceMmALQPYBW/Xltxn0NAMnU= +github.com/klauspost/compress v1.17.8/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= +github.com/philhofer/fwd v1.1.2 h1:bnDivRJ1EWPjUIRXV5KfORO897HTbpFAQddBdE8t7Gw= +github.com/philhofer/fwd v1.1.2/go.mod h1:qkPdfjR2SIEbspLqpe1tO4n5yICnr2DY7mqEx2tUTP0= +github.com/tinylib/msgp v1.1.9 h1:SHf3yoO2sGA0veCJeCBYLHuttAVFHGm2RHgNodW7wQU= +github.com/tinylib/msgp v1.1.9/go.mod h1:BCXGB54lDD8qUEPmiG0cQQUANC4IUQyB2ItS2UDlO/k= diff --git a/reader.go b/reader.go index 004c199..beecd8f 100644 --- a/reader.go +++ b/reader.go @@ -558,6 +558,11 @@ func readDirectoryEnd(buf []byte, size int64) (dir *directoryEnd, err error) { return nil, err } } + maxInt64 := uint64(1<<63 - 1) + if d.directorySize > maxInt64 || d.directoryOffset > maxInt64 { + return nil, ErrFormat + } + // Make sure directoryOffset points to somewhere in our file. if o := int64(d.directoryOffset); o < 0 || o >= size { return nil, ErrFormat @@ -617,9 +622,13 @@ func findSignatureInBlock(b []byte) int { if b[i] == 'P' && b[i+1] == 'K' && b[i+2] == 0x05 && b[i+3] == 0x06 { // n is length of comment n := int(b[i+directoryEndLen-2]) | int(b[i+directoryEndLen-1])<<8 - if n+directoryEndLen+i <= len(b) { - return i + if n+directoryEndLen+i > len(b) { + // Truncated comment. + // Some parsers (such as Info-ZIP) ignore the truncated comment + // rather than treating it as a hard error. + return -1 } + return i } } return -1 diff --git a/testdata/comment-truncated.zip b/testdata/comment-truncated.zip new file mode 100644 index 0000000000000000000000000000000000000000..1bc19a85575964f378a8a30f198ed6ba5360aa7d GIT binary patch literal 216 zcmWIWW@cf4gUWrGI~jpI5C#dmdHT2ppr}ax{CO=%28Pozb5c_hOA-UT8JU2>aDc83 pE&*nMbOm^`vVk~^KxhP{)xa|7=AgR>tO!m(+=pt;1fVP<0{|c)7RUeq literal 0 HcmV?d00001 diff --git a/testdata/readme.notzip b/testdata/readme.notzip index 81737275c6ebf5ea69b992753ab4050f031f31b8..79b1cb6de33c6ae86451acedbd50df4207a5710e 100644 GIT binary patch delta 15 Wcmeyw_la-AHzp?UfXzRcs#pLyiw1E3 delta 15 Wcmeyw_la-AHzuY4@6A7$s#pLykOpx8 diff --git a/testdata/test-badbase.zip b/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}l&aaaM)^pwV1FgBTd*)~VY5GrSri z$jrb1!XgYZ4C(m=8L36d`8oMThGrFpW_ksA>0oQD44Qqcff&u2&Hz7mUM?w+fxMm` zE&U=x?Zy@V2qPe0vcxr_Bsf2F!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(96e0boH{+WCE(bu95f2^3EZ4O=VNZ?3)UvtyJg5QDRcf$^EI~Jki zWtQzJsL_x*GnsW)scd27+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#tAeEd>m$^DR^;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=23UUDy$e_XNHK(DWELv`QKCXsW Date: Wed, 5 Jun 2024 15:20:28 +0200 Subject: [PATCH 2/4] Update golangci-lint --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 8cea6b2..fad12a1 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -30,6 +30,6 @@ jobs: env: GO111MODULE: on run: | - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.45.0 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.59.0 $(go env GOPATH)/bin/golangci-lint run --timeout=5m --config ./.golangci.yml go test -race ./... From be478ac742949ae0b5463b267c05e3bb035ef53b Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Wed, 5 Jun 2024 15:27:22 +0200 Subject: [PATCH 3/4] Update lint tests --- .golangci.yml | 7 +++---- example_test.go | 7 +++---- fuzz_test.go | 3 +-- reader.go | 7 +++---- register.go | 3 +-- struct.go | 8 -------- zipindex_test.go | 9 +++++---- 7 files changed, 16 insertions(+), 28 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d15ba92..1991202 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,11 +12,10 @@ linters: - goimports - misspell - govet - - golint - ineffassign - gosimple - - deadcode - - structcheck + - staticcheck + - unused issues: exclude-use-default: false @@ -26,4 +25,4 @@ issues: - should have comment or be unexported - error strings should not be capitalized or end with punctuation or a newline service: - golangci-lint-version: 1.20.0 # use the fixed version to not introduce new linters unexpectedly + golangci-lint-version: 1.59.0 # use the fixed version to not introduce new linters unexpectedly diff --git a/example_test.go b/example_test.go index 289643d..47f31f2 100644 --- a/example_test.go +++ b/example_test.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "log" "os" @@ -29,7 +28,7 @@ import ( ) func ExampleReadDir() { - b, err := ioutil.ReadFile("testdata/big.zip") + b, err := os.ReadFile("testdata/big.zip") if err != nil { panic(err) } @@ -160,7 +159,7 @@ func ExampleDeserializeFiles() { } } - b, err := ioutil.ReadFile("testdata/big.zip") + b, err := os.ReadFile("testdata/big.zip") exitOnErr(err) // We only need the end of the file to parse the directory. // Usually this should be at least 64K on initial try. @@ -212,7 +211,7 @@ func ExampleDeserializeFiles() { defer rc.Close() // Read the zip file content. - content, err := ioutil.ReadAll(rc) + content, err := io.ReadAll(rc) exitOnErr(err) fmt.Printf("File content is '%s'\n", string(content)) diff --git a/fuzz_test.go b/fuzz_test.go index e5f8a27..052fce9 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -28,7 +28,6 @@ import ( "go/parser" "go/token" "io" - "io/ioutil" "os" "strconv" "testing" @@ -96,7 +95,7 @@ func FuzzRoundtrip(f *testing.F) { defer rc.Close() // Read the zip file content. - ioutil.ReadAll(rc) + io.ReadAll(rc) } }) } diff --git a/reader.go b/reader.go index beecd8f..e578011 100644 --- a/reader.go +++ b/reader.go @@ -29,7 +29,6 @@ import ( "fmt" "hash" "io" - "io/ioutil" "os" "time" "unicode/utf8" @@ -206,7 +205,7 @@ func ReadFile(name string, filter FileFilter) (Files, error) { if err != nil { return nil, err } - b, err := ioutil.ReadAll(f) + b, err := io.ReadAll(f) if err != nil { return nil, err } @@ -254,7 +253,7 @@ func (r *checksumReader) Read(b []byte) (n int, err error) { if r.f.hasDataDescriptor() { // If any compressed data remains, read it. - io.Copy(ioutil.Discard, r.compReader) + io.Copy(io.Discard, r.compReader) if err1 := readDataDescriptor(r.raw, r.f); err1 != nil { if err1 == io.EOF { @@ -299,7 +298,7 @@ func (f *File) skipToBody(r io.Reader) error { filenameLen := int(b.uint16()) extraLen := int(b.uint16()) // Skip extra... - _, err := io.CopyN(ioutil.Discard, r, int64(filenameLen+extraLen)) + _, err := io.CopyN(io.Discard, r, int64(filenameLen+extraLen)) return err } diff --git a/register.go b/register.go index 3260590..5a415d3 100644 --- a/register.go +++ b/register.go @@ -25,7 +25,6 @@ package zipindex import ( "errors" "io" - "io/ioutil" "sync" "github.com/klauspost/compress/flate" @@ -82,7 +81,7 @@ var ( ) func init() { - RegisterDecompressor(Store, ioutil.NopCloser) + RegisterDecompressor(Store, io.NopCloser) RegisterDecompressor(Deflate, newFlateReader) RegisterDecompressor(Zstd, zstd.ZipDecompressor(zstd.WithDecoderLowmem(true))) } diff --git a/struct.go b/struct.go index 0b5003a..6a299e7 100644 --- a/struct.go +++ b/struct.go @@ -58,9 +58,6 @@ const ( // Version numbers. - // Limits for non zip64 files. - uint32max = (1 << 32) - 1 - // Extra header IDs. // // IDs 0..31 are reserved for official use by PKWARE. @@ -199,11 +196,6 @@ func (h *ZipDirEntry) Mode() (mode os.FileMode) { return mode } -// isZip64 reports whether the file size exceeds the 32 bit limit -func (h *ZipDirEntry) isZip64() bool { - return h.CompressedSize64 >= uint32max || h.UncompressedSize64 >= uint32max -} - func msdosModeToFileMode(m uint32) (mode os.FileMode) { if m&msdosDir != 0 { mode = os.ModeDir | 0777 diff --git a/zipindex_test.go b/zipindex_test.go index 7466f3f..ed77be9 100644 --- a/zipindex_test.go +++ b/zipindex_test.go @@ -20,7 +20,8 @@ import ( "archive/zip" "bytes" "errors" - "io/ioutil" + "io" + "os" "path/filepath" "reflect" "testing" @@ -67,7 +68,7 @@ func TestReadDir(t *testing.T) { } for _, test := range testSet { t.Run(test, func(t *testing.T) { - input, err := ioutil.ReadFile(filepath.Join("testdata", test)) + input, err := os.ReadFile(filepath.Join("testdata", test)) if err != nil { t.Fatal(err) } @@ -161,8 +162,8 @@ func TestReadDir(t *testing.T) { t.Errorf("err mismatch: %v != %v", wantErr, gotErr) } }() - wantData, wantErr := ioutil.ReadAll(wantRC) - gotData, err := ioutil.ReadAll(rc) + wantData, wantErr := io.ReadAll(wantRC) + gotData, err := io.ReadAll(rc) if err != nil { if err == wantErr { continue From b544b9d3f0956818e5b6e87d4271d8f9f853af86 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Wed, 5 Jun 2024 15:36:57 +0200 Subject: [PATCH 4/4] Remove Go 1.20, since it will not have the fix backported for the test to pass. --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index fad12a1..3a09960 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -14,7 +14,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - go-version: [1.20.x,1.21.x,1.22.x] + go-version: [1.21.x,1.22.x] os: [ubuntu-latest] steps: - name: Set up Go ${{ matrix.go-version }} on ${{ matrix.os }}