From bb9304ea23d262204a61eedbb810d60b4cb4e01e Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Tue, 16 Jan 2024 12:47:55 -0600 Subject: [PATCH 1/5] pkcs7: fix slice out-of-bounds panic --- helper/pkcs7/ber.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helper/pkcs7/ber.go b/helper/pkcs7/ber.go index 0b18a6c8d361..eb6b1d0af660 100644 --- a/helper/pkcs7/ber.go +++ b/helper/pkcs7/ber.go @@ -149,14 +149,14 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) { for ber[offset] >= 0x80 { tag = tag*128 + ber[offset] - 0x80 offset++ - if offset > berLen { + if offset >= berLen { return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached") } } // jvehent 20170227: this doesn't appear to be used anywhere... // tag = tag*128 + ber[offset] - 0x80 offset++ - if offset > berLen { + if offset >= berLen { return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached") } } @@ -172,7 +172,7 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) { var length int l := ber[offset] offset++ - if offset > berLen { + if offset >= berLen { return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached") } indefinite := false @@ -192,7 +192,7 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) { for i := 0; i < numberOfBytes; i++ { length = length*256 + (int)(ber[offset]) offset++ - if offset > berLen { + if offset >= berLen { return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached") } } From 3f4f1eb67555a06d102f9eb8f027fea2cd9ab7a0 Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Tue, 16 Jan 2024 12:48:53 -0600 Subject: [PATCH 2/5] changelog --- changelog/24891.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/24891.txt diff --git a/changelog/24891.txt b/changelog/24891.txt new file mode 100644 index 000000000000..6f84e14290a5 --- /dev/null +++ b/changelog/24891.txt @@ -0,0 +1,3 @@ +```release-note:bug +helper/pkcs7: Fix slice out-of-bounds panic +``` From bee6d3500c13910d79b6fc4c3df6d530149c0124 Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Tue, 16 Jan 2024 13:58:38 -0600 Subject: [PATCH 3/5] fix tests --- helper/pkcs7/ber_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helper/pkcs7/ber_test.go b/helper/pkcs7/ber_test.go index 169c78ab701e..a51082ac88de 100644 --- a/helper/pkcs7/ber_test.go +++ b/helper/pkcs7/ber_test.go @@ -44,11 +44,11 @@ func TestBer2Der_Negatives(t *testing.T) { Input []byte ErrorContains string }{ - {[]byte{0x30, 0x85}, "tag length too long"}, + {[]byte{0x30, 0x85}, "end of ber data reached"}, {[]byte{0x30, 0x84, 0x80, 0x0, 0x0, 0x0}, "length is negative"}, {[]byte{0x30, 0x82, 0x0, 0x1}, "length has leading zero"}, {[]byte{0x30, 0x80, 0x1, 0x2, 0x1, 0x2}, "Invalid BER format"}, - {[]byte{0x30, 0x80, 0x1, 0x2}, "BER tag length is more than available data"}, + {[]byte{0x30, 0x80, 0x1, 0x2}, "end of ber data reached"}, {[]byte{0x30, 0x03, 0x01, 0x02}, "length is more than available data"}, {[]byte{0x30}, "end of ber data reached"}, } From 52775da5ce8d1f3808c9c81b0621962a99ea841a Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Tue, 16 Jan 2024 15:03:06 -0600 Subject: [PATCH 4/5] add test case to capture panic; found in fuzzing --- helper/pkcs7/ber_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/helper/pkcs7/ber_test.go b/helper/pkcs7/ber_test.go index a51082ac88de..2af535b0c846 100644 --- a/helper/pkcs7/ber_test.go +++ b/helper/pkcs7/ber_test.go @@ -51,6 +51,7 @@ func TestBer2Der_Negatives(t *testing.T) { {[]byte{0x30, 0x80, 0x1, 0x2}, "end of ber data reached"}, {[]byte{0x30, 0x03, 0x01, 0x02}, "length is more than available data"}, {[]byte{0x30}, "end of ber data reached"}, + {[]byte("?0"), "end of ber data reached"}, } for _, fixture := range fixtures { From 8799c1b68f8f6880ee4c624aba5c27b6441580fd Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Wed, 17 Jan 2024 09:18:49 -0600 Subject: [PATCH 5/5] add fuzz test --- helper/pkcs7/ber_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/helper/pkcs7/ber_test.go b/helper/pkcs7/ber_test.go index 2af535b0c846..d3908f6bc32c 100644 --- a/helper/pkcs7/ber_test.go +++ b/helper/pkcs7/ber_test.go @@ -9,6 +9,38 @@ import ( "testing" ) +// FuzzReadObject is a fuzz test that will generate random input data in an +// attempt to find crash-causing inputs +// https://go.dev/doc/security/fuzz +func FuzzReadObject(f *testing.F) { + // seed corpus used to guide the fuzzing engine + seedCorpus := []struct { + input []byte + offset int + }{ + {[]byte{0x30, 0x85}, 0}, + {[]byte{0x30, 0x84, 0x80, 0x0, 0x0, 0x0}, 0}, + {[]byte{0x30, 0x82, 0x0, 0x1}, 0}, + {[]byte{0x30, 0x80, 0x1, 0x2, 0x1, 0x2}, 0}, + {[]byte{0x30, 0x80, 0x1, 0x2}, 0}, + {[]byte{0x30, 0x03, 0x01, 0x02}, 0}, + {[]byte{0x30}, 0}, + {[]byte("?0"), 0}, + } + for _, tc := range seedCorpus { + f.Add(tc.input, tc.offset) // Use f.Add to provide a seed corpus + } + f.Fuzz(func(t *testing.T, ber []byte, offset int) { + if offset < 0 { + return + } + _, _, err := readObject(ber, offset) + if err != nil { + t.Log(ber, offset) + } + }) +} + func TestBer2Der(t *testing.T) { // indefinite length fixture ber := []byte{0x30, 0x80, 0x02, 0x01, 0x01, 0x00, 0x00}