From c8e40338cf00f3c1d86c8fb23863ad67a4c72bcc Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 14 May 2024 14:39:10 -0700 Subject: [PATCH] [release-branch.go1.21] archive/zip: treat truncated EOCDR comment as an error When scanning for an end of central directory record, treat an EOCDR signature with a record containing a truncated comment as an error. Previously, we would skip over the invalid record and look for another one. Other implementations do not do this (they either consider this a hard error, or just ignore the truncated comment). This parser misalignment allowed presenting entirely different archive contents to Go programs and other zip decoders. For #66869 Fixes #67553 Change-Id: I94e5cb028534bb5704588b8af27f1e22ea49c7c6 Reviewed-on: https://go-review.googlesource.com/c/go/+/585397 Reviewed-by: Joseph Tsai Reviewed-by: Dmitri Shuralyov LUCI-TryBot-Result: Go LUCI (cherry picked from commit 33d725e5758bf1fea62e6c77fc70b57a828a49f5) Reviewed-on: https://go-review.googlesource.com/c/go/+/588795 Reviewed-by: Matthew Dempsky CVE: CVE-2024-24789 Upstream-Status: Backport [https://github.com/golang/go/commit/c8e40338cf00f3c1d86c8fb23863ad67a4c72bcc] Signed-off-by: Soumya Sambu --- src/archive/zip/reader.go | 8 ++++++-- src/archive/zip/reader_test.go | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go index e40a2c6..987f543 100644 --- a/src/archive/zip/reader.go +++ b/src/archive/zip/reader.go @@ -644,9 +644,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/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go index a549153..7ac394d 100644 --- a/src/archive/zip/reader_test.go +++ b/src/archive/zip/reader_test.go @@ -487,6 +487,14 @@ var tests = []ZipTest{ }, }, }, + // Issue 66869: Don't skip over an EOCDR with a truncated comment. + // The test file sneakily hides a second EOCDR before the first one; + // previously we would extract one file ("file") from this archive, + // while most other tools would reject the file or extract a different one ("FILE"). + { + Name: "comment-truncated.zip", + Error: ErrFormat, + }, } func TestReader(t *testing.T) { -- 2.40.0