Skip to content

Commit 3fffae9

Browse files
authored
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
1 parent 80f85a0 commit 3fffae9

File tree

3 files changed

+42
-6
lines changed

3 files changed

+42
-6
lines changed

changelog/24891.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
helper/pkcs7: Fix slice out-of-bounds panic
3+
```

helper/pkcs7/ber.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,14 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) {
149149
for ber[offset] >= 0x80 {
150150
tag = tag*128 + ber[offset] - 0x80
151151
offset++
152-
if offset > berLen {
152+
if offset >= berLen {
153153
return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
154154
}
155155
}
156156
// jvehent 20170227: this doesn't appear to be used anywhere...
157157
// tag = tag*128 + ber[offset] - 0x80
158158
offset++
159-
if offset > berLen {
159+
if offset >= berLen {
160160
return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
161161
}
162162
}
@@ -172,7 +172,7 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) {
172172
var length int
173173
l := ber[offset]
174174
offset++
175-
if offset > berLen {
175+
if offset >= berLen {
176176
return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
177177
}
178178
indefinite := false
@@ -192,7 +192,7 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) {
192192
for i := 0; i < numberOfBytes; i++ {
193193
length = length*256 + (int)(ber[offset])
194194
offset++
195-
if offset > berLen {
195+
if offset >= berLen {
196196
return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
197197
}
198198
}

helper/pkcs7/ber_test.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,38 @@ import (
99
"testing"
1010
)
1111

12+
// FuzzReadObject is a fuzz test that will generate random input data in an
13+
// attempt to find crash-causing inputs
14+
// https://go.dev/doc/security/fuzz
15+
func FuzzReadObject(f *testing.F) {
16+
// seed corpus used to guide the fuzzing engine
17+
seedCorpus := []struct {
18+
input []byte
19+
offset int
20+
}{
21+
{[]byte{0x30, 0x85}, 0},
22+
{[]byte{0x30, 0x84, 0x80, 0x0, 0x0, 0x0}, 0},
23+
{[]byte{0x30, 0x82, 0x0, 0x1}, 0},
24+
{[]byte{0x30, 0x80, 0x1, 0x2, 0x1, 0x2}, 0},
25+
{[]byte{0x30, 0x80, 0x1, 0x2}, 0},
26+
{[]byte{0x30, 0x03, 0x01, 0x02}, 0},
27+
{[]byte{0x30}, 0},
28+
{[]byte("?0"), 0},
29+
}
30+
for _, tc := range seedCorpus {
31+
f.Add(tc.input, tc.offset) // Use f.Add to provide a seed corpus
32+
}
33+
f.Fuzz(func(t *testing.T, ber []byte, offset int) {
34+
if offset < 0 {
35+
return
36+
}
37+
_, _, err := readObject(ber, offset)
38+
if err != nil {
39+
t.Log(ber, offset)
40+
}
41+
})
42+
}
43+
1244
func TestBer2Der(t *testing.T) {
1345
// indefinite length fixture
1446
ber := []byte{0x30, 0x80, 0x02, 0x01, 0x01, 0x00, 0x00}
@@ -44,13 +76,14 @@ func TestBer2Der_Negatives(t *testing.T) {
4476
Input []byte
4577
ErrorContains string
4678
}{
47-
{[]byte{0x30, 0x85}, "tag length too long"},
79+
{[]byte{0x30, 0x85}, "end of ber data reached"},
4880
{[]byte{0x30, 0x84, 0x80, 0x0, 0x0, 0x0}, "length is negative"},
4981
{[]byte{0x30, 0x82, 0x0, 0x1}, "length has leading zero"},
5082
{[]byte{0x30, 0x80, 0x1, 0x2, 0x1, 0x2}, "Invalid BER format"},
51-
{[]byte{0x30, 0x80, 0x1, 0x2}, "BER tag length is more than available data"},
83+
{[]byte{0x30, 0x80, 0x1, 0x2}, "end of ber data reached"},
5284
{[]byte{0x30, 0x03, 0x01, 0x02}, "length is more than available data"},
5385
{[]byte{0x30}, "end of ber data reached"},
86+
{[]byte("?0"), "end of ber data reached"},
5487
}
5588

5689
for _, fixture := range fixtures {

0 commit comments

Comments
 (0)