From 3fffae9452588067c0cf652717b6ace17fdfdfe4 Mon Sep 17 00:00:00 2001
From: John-Michael Faircloth <fairclothjm@users.noreply.github.com>
Date: Wed, 17 Jan 2024 10:12:00 -0600
Subject: [PATCH] pkcs7: fix slice out-of-bounds panic (#24891)

* pkcs7: fix slice out-of-bounds panic

* changelog

* fix tests

* add test case to capture panic; found in fuzzing

* add fuzz test
---
 changelog/24891.txt      |  3 +++
 helper/pkcs7/ber.go      |  8 ++++----
 helper/pkcs7/ber_test.go | 37 +++++++++++++++++++++++++++++++++++--
 3 files changed, 42 insertions(+), 6 deletions(-)
 create mode 100644 changelog/24891.txt

diff --git a/changelog/24891.txt b/changelog/24891.txt
new file mode 100644
index 0000000000..6f84e14290
--- /dev/null
+++ b/changelog/24891.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+helper/pkcs7: Fix slice out-of-bounds panic
+```
diff --git a/helper/pkcs7/ber.go b/helper/pkcs7/ber.go
index 0b18a6c8d3..eb6b1d0af6 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")
 			}
 		}
diff --git a/helper/pkcs7/ber_test.go b/helper/pkcs7/ber_test.go
index 169c78ab70..d3908f6bc3 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}
@@ -44,13 +76,14 @@ 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"},
+		{[]byte("?0"), "end of ber data reached"},
 	}
 
 	for _, fixture := range fixtures {