From 3562fcf934a0475f35bd29112bb2999a700a213a Mon Sep 17 00:00:00 2001 From: Heitor Neiva Date: Tue, 21 Jul 2026 09:41:09 -0700 Subject: [PATCH] Fix out-of-bounds panic in ber2der on malformed BER input `pkcs7.Parse()` could panic with out-of-bounds slice/index reads when given certain tiny malformed BER inputs, instead of returning an error (CWE-125 / CWE-193). `readObject()` in ber.go guarded its cursor with `offset > berLen` rather than `>=`, letting `offset` reach `berLen` and still pass, so the subsequent `ber[offset]` read one past the end of the slice. The long-form length branch also sliced `ber[offset:offset+numberOfBytes]` with no upper-bound check. - Change the high-tag-number guards to `offset >= berLen` - Add an `offset+numberOfBytes > berLen` check before reading the long-form length octets. Malformed inputs such as {1F 80}, {1F 05}, {30 81}, and {30 84 01} now return an error instead of panicking. Valid BER/DER, including empty definite-length objects and high-tag-number tags, is unaffected. Add regression tests covering the malformed inputs (via ber2der and the exported `Parse` entry point) and a positive round-trip test for high-tag-number encodings. --- ber.go | 7 +++++-- ber_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/ber.go b/ber.go index 0cc2311..5688961 100644 --- a/ber.go +++ b/ber.go @@ -140,14 +140,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,6 +172,9 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) { if numberOfBytes > 4 { // int is only guaranteed to be 32bit return nil, 0, errors.New("ber2der: BER tag length too long") } + if offset+numberOfBytes > berLen { + return nil, 0, errors.New("ber2der: BER tag length is more than available data") + } if numberOfBytes == 4 && (int)(ber[offset]) > 0x7F { return nil, 0, errors.New("ber2der: BER tag length is negative") } diff --git a/ber_test.go b/ber_test.go index 7e21149..640daae 100644 --- a/ber_test.go +++ b/ber_test.go @@ -53,6 +53,11 @@ func TestBer2Der_Negatives(t *testing.T) { {[]byte{0x30, 0x80, 0x1, 0x2}, "BER tag length is more than available data"}, {[]byte{0x30, 0x03, 0x01, 0x02}, "length is more than available data"}, {[]byte{0x30}, "end of ber data reached"}, + // GHSA-mq3g-qwhv-4hgw: malformed BER that previously caused out-of-bounds panics. + {[]byte{0x1F, 0x80}, "end of ber data reached"}, // high-tag continuation byte is last + {[]byte{0x1F, 0x05}, "end of ber data reached"}, // high-tag ends with no length octet + {[]byte{0x30, 0x81}, "more than available data"}, // long-form length indicator is last + {[]byte{0x30, 0x84, 0x01}, "more than available data"}, // declares 4 length bytes, only 1 present } for _, fixture := range fixtures { @@ -66,6 +71,44 @@ func TestBer2Der_Negatives(t *testing.T) { } } +func TestParseMalformedBERNoPanic(t *testing.T) { + t.Parallel() + // GHSA-mq3g-qwhv-4hgw: these tiny malformed inputs used to panic Parse via + // out-of-bounds reads in ber2der/readObject. They must now return an error. + fixtures := [][]byte{ + {0x1F, 0x80}, + {0x1F, 0x05}, + {0x30, 0x81}, + {0x30, 0x84, 0x01}, + } + for _, in := range fixtures { + if _, err := Parse(in); err == nil { + t.Errorf("Parse(% X): expected error, got nil", in) + } + } +} + +func TestBer2Der_HighTagNumber(t *testing.T) { + t.Parallel() + // Valid high-tag-number (0x1F prefix) encodings must still round-trip after + // the bounds-check tightening in readObject. These inputs are already DER, so + // ber2der should return them unchanged. + fixtures := [][]byte{ + {0x1F, 0x05, 0x02, 0x01, 0x07}, // single tag-number octet + {0x1F, 0x81, 0x00, 0x01, 0x41}, // multi-byte tag number (128), exercises the continuation loop + } + for _, in := range fixtures { + der, err := ber2der(in) + if err != nil { + t.Errorf("ber2der(% X) failed: %v", in, err) + continue + } + if !bytes.Equal(der, in) { + t.Errorf("ber2der(% X) = % X, want unchanged", in, der) + } + } +} + func TestBer2Der_NestedMultipleIndefinite(t *testing.T) { t.Parallel() // indefinite length fixture