Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport fix for CVE-2024-24789 #223

Open
wants to merge 1 commit into
base: go1.19-fips-release
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions patches/012-fix-CVE-2024-24789.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
From 33d725e5758bf1fea62e6c77fc70b57a828a49f5 Mon Sep 17 00:00:00 2001
From: Damien Neil <[email protected]>
Date: Tue, 14 May 2024 14:39:10 -0700
Subject: [PATCH] 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.

Fixes #66869

Change-Id: I94e5cb028534bb5704588b8af27f1e22ea49c7c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/585397
Reviewed-by: Joseph Tsai <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
---
src/archive/zip/reader.go | 8 ++++++--
src/archive/zip/reader_test.go | 8 ++++++++
src/archive/zip/testdata/comment-truncated.zip | Bin 0 -> 216 bytes
3 files changed, 14 insertions(+), 2 deletions(-)
create mode 100644 src/archive/zip/testdata/comment-truncated.zip

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 9f651da530..00e5ec3e05 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
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

--
2.46.0

Loading