diff options
| author | Damien Neil <dneil@google.com> | 2024-05-14 14:39:10 -0700 |
|---|---|---|
| committer | Carlos Amedee <carlos@golang.org> | 2024-05-29 23:37:37 +0000 |
| commit | cf501ac0c5fe351a8582d20b43562027927906e7 (patch) | |
| tree | 66e1af8e55af212cd126c5424c046c5699a4d340 | |
| parent | cb55d1a0c8e37c4e5c3c45dc6e8fed8d76a18b90 (diff) | |
| download | go-cf501ac0c5fe351a8582d20b43562027927906e7.tar.xz | |
[release-branch.go1.22] 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 #67554
Change-Id: I94e5cb028534bb5704588b8af27f1e22ea49c7c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/585397
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 33d725e5758bf1fea62e6c77fc70b57a828a49f5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/588796
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
| -rw-r--r-- | src/archive/zip/reader.go | 8 | ||||
| -rw-r--r-- | src/archive/zip/reader_test.go | 8 | ||||
| -rw-r--r-- | src/archive/zip/testdata/comment-truncated.zip | bin | 0 -> 216 bytes |
3 files changed, 14 insertions, 2 deletions
diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go index ff6fedf632..60b34b76ee 100644 --- a/src/archive/zip/reader.go +++ b/src/archive/zip/reader.go @@ -699,9 +699,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 631515cf5d..9a77c1aa62 100644 --- a/src/archive/zip/reader_test.go +++ b/src/archive/zip/reader_test.go @@ -570,6 +570,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) { diff --git a/src/archive/zip/testdata/comment-truncated.zip b/src/archive/zip/testdata/comment-truncated.zip Binary files differnew file mode 100644 index 0000000000..1bc19a8557 --- /dev/null +++ b/src/archive/zip/testdata/comment-truncated.zip |