diff --git a/ber.go b/ber.go index 5233321..d6fc54c 100644 --- a/ber.go +++ b/ber.go @@ -101,12 +101,12 @@ func lengthLength(i int) (numBytes int) { // added to 0x80. The length is encoded in big endian encoding follow after // // Examples: -// length | byte 1 | bytes n -// 0 | 0x00 | - -// 120 | 0x78 | - -// 200 | 0x81 | 0xC8 -// 500 | 0x82 | 0x01 0xF4 // +// length | byte 1 | bytes n +// 0 | 0x00 | - +// 120 | 0x78 | - +// 200 | 0x81 | 0xC8 +// 500 | 0x82 | 0x01 0xF4 func encodeLength(out *bytes.Buffer, length int) (err error) { if length >= 128 { l := lengthLength(length) @@ -144,14 +144,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") } } @@ -176,6 +176,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 f2ba5d2..7690a44 100644 --- a/ber_test.go +++ b/ber_test.go @@ -51,6 +51,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 { @@ -64,6 +69,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) { // indefinite length fixture ber := []byte{0x30, 0x80, 0x30, 0x80, 0x02, 0x01, 0x01, 0x00, 0x00, 0x30, 0x80, 0x02, 0x01, 0x02, 0x00, 0x00, 0x00, 0x00}