|
| 1 | +From 33d725e5758bf1fea62e6c77fc70b57a828a49f5 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Damien Neil < [email protected]> |
| 3 | +Date: Tue, 14 May 2024 14:39:10 -0700 |
| 4 | +Subject: [PATCH] archive/zip: treat truncated EOCDR comment as an error |
| 5 | + |
| 6 | +When scanning for an end of central directory record, |
| 7 | +treat an EOCDR signature with a record containing a truncated |
| 8 | +comment as an error. Previously, we would skip over the invalid |
| 9 | +record and look for another one. Other implementations do not |
| 10 | +do this (they either consider this a hard error, or just ignore |
| 11 | +the truncated comment). This parser misalignment allowed |
| 12 | +presenting entirely different archive contents to Go programs |
| 13 | +and other zip decoders. |
| 14 | + |
| 15 | +Fixes #66869 |
| 16 | + |
| 17 | +Change-Id: I94e5cb028534bb5704588b8af27f1e22ea49c7c6 |
| 18 | +Reviewed-on: https://go-review.googlesource.com/c/go/+/585397 |
| 19 | +Reviewed-by: Joseph Tsai < [email protected]> |
| 20 | +Reviewed-by: Dmitri Shuralyov < [email protected]> |
| 21 | +LUCI-TryBot-Result: Go LUCI < [email protected]> |
| 22 | +--- |
| 23 | + src/archive/zip/reader.go | 8 ++++++-- |
| 24 | + src/archive/zip/reader_test.go | 8 ++++++++ |
| 25 | + src/archive/zip/testdata/comment-truncated.zip | Bin 0 -> 216 bytes |
| 26 | + 3 files changed, 14 insertions(+), 2 deletions(-) |
| 27 | + create mode 100644 src/archive/zip/testdata/comment-truncated.zip |
| 28 | + |
| 29 | +diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go |
| 30 | +index ff6fedf632..60b34b76ee 100644 |
| 31 | +--- a/src/archive/zip/reader.go |
| 32 | ++++ b/src/archive/zip/reader.go |
| 33 | +@@ -699,9 +699,13 @@ func findSignatureInBlock(b []byte) int { |
| 34 | + if b[i] == 'P' && b[i+1] == 'K' && b[i+2] == 0x05 && b[i+3] == 0x06 { |
| 35 | + // n is length of comment |
| 36 | + n := int(b[i+directoryEndLen-2]) | int(b[i+directoryEndLen-1])<<8 |
| 37 | +- if n+directoryEndLen+i <= len(b) { |
| 38 | +- return i |
| 39 | ++ if n+directoryEndLen+i > len(b) { |
| 40 | ++ // Truncated comment. |
| 41 | ++ // Some parsers (such as Info-ZIP) ignore the truncated comment |
| 42 | ++ // rather than treating it as a hard error. |
| 43 | ++ return -1 |
| 44 | + } |
| 45 | ++ return i |
| 46 | + } |
| 47 | + } |
| 48 | + return -1 |
| 49 | +diff --git a/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go |
| 50 | +index 9f651da530..00e5ec3e05 100644 |
| 51 | +--- a/src/archive/zip/reader_test.go |
| 52 | ++++ b/src/archive/zip/reader_test.go |
| 53 | +@@ -570,6 +570,14 @@ var tests = []ZipTest{ |
| 54 | + }, |
| 55 | + }, |
| 56 | + }, |
| 57 | ++ // Issue 66869: Don't skip over an EOCDR with a truncated comment. |
| 58 | ++ // The test file sneakily hides a second EOCDR before the first one; |
| 59 | ++ // previously we would extract one file ("file") from this archive, |
| 60 | ++ // while most other tools would reject the file or extract a different one ("FILE"). |
| 61 | ++ { |
| 62 | ++ Name: "comment-truncated.zip", |
| 63 | ++ Error: ErrFormat, |
| 64 | ++ }, |
| 65 | + } |
| 66 | + |
| 67 | + func TestReader(t *testing.T) { |
| 68 | +diff --git a/src/archive/zip/testdata/comment-truncated.zip b/src/archive/zip/testdata/comment-truncated.zip |
| 69 | +new file mode 100644 |
| 70 | +index 0000000000000000000000000000000000000000..1bc19a85575964f378a8a30f198ed6ba5360aa7d |
| 71 | +GIT binary patch |
| 72 | +literal 216 |
| 73 | +zcmWIWW@cf4gUWrGI~jpI5C#dmdHT2ppr}ax{CO=%28Pozb5c_hOA-UT8JU2>aDc83 |
| 74 | +pE&*nMbOm^`vVk~^KxhP{)xa|7=AgR>tO!m(+=pt;1fVP<0{|c)7RUeq |
| 75 | + |
| 76 | +literal 0 |
| 77 | +HcmV?d00001 |
| 78 | + |
| 79 | +-- |
| 80 | +2.46.0 |
| 81 | + |
0 commit comments