From b9b61dfa86186ec1b04b674db2c7239e8fd004c2 Mon Sep 17 00:00:00 2001 From: Nikita Kryukov <25217914+Cravtos@users.noreply.github.com> Date: Tue, 14 May 2024 12:17:25 +0700 Subject: [PATCH 1/2] fix parser out of bounds access panics --- ber.go | 10 +++++----- ber_test.go | 5 +++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/ber.go b/ber.go index 73da024..16020b9 100644 --- a/ber.go +++ b/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 @@ -188,11 +188,11 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) { return nil, 0, errors.New("ber2der: BER tag length has leading zero") } debugprint("--> (compute length) indicator byte: %x\n", l) - debugprint("--> (compute length) length bytes: % X\n", ber[offset:offset+numberOfBytes]) + //debugprint("--> (compute length) length bytes: % X\n", ber[offset:offset+numberOfBytes]) // may panic if unoptimized 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/ber_test.go b/ber_test.go index f2ba5d2..0a818a8 100644 --- a/ber_test.go +++ b/ber_test.go @@ -44,13 +44,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 0 0 0 0 0 0 0\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x02\x1f0"), "end of ber data reached"}, } for _, fixture := range fixtures { From b7da01dc92d85bd571074f0be34006394aec6486 Mon Sep 17 00:00:00 2001 From: Nikita Kryukov <25217914+Cravtos@users.noreply.github.com> Date: Thu, 23 May 2024 23:32:08 +0700 Subject: [PATCH 2/2] Remove redundant check length check Add tests to cover all length check cases --- ber.go | 15 ++++++--------- ber_test.go | 10 ++++++---- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/ber.go b/ber.go index 16020b9..a47e592 100644 --- a/ber.go +++ b/ber.go @@ -106,12 +106,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) @@ -134,9 +134,6 @@ func encodeLength(out *bytes.Buffer, length int) (err error) { func readObject(ber []byte, offset int) (asn1Object, int, error) { berLen := len(ber) - if offset >= berLen { - return nil, 0, errors.New("ber2der: offset is after end of ber data") - } tagStart := offset b := ber[offset] offset++ @@ -259,7 +256,7 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) { } func isIndefiniteTermination(ber []byte, offset int) (bool, error) { - if len(ber) - offset < 2 { + if len(ber)-offset < 2 { return false, errors.New("ber2der: Invalid BER format") } diff --git a/ber_test.go b/ber_test.go index 0a818a8..27c471d 100644 --- a/ber_test.go +++ b/ber_test.go @@ -44,14 +44,16 @@ func TestBer2Der_Negatives(t *testing.T) { Input []byte ErrorContains string }{ - {[]byte{0x30, 0x85}, "end of ber data reached"}, + {[]byte{0x1f, 0x0}, "end of ber data reached"}, + {[]byte{0x30, 0x85, 0x1}, "tag length too long"}, {[]byte{0x30, 0x84, 0x80, 0x0, 0x0, 0x0}, "length is negative"}, {[]byte{0x30, 0x82, 0x0, 0x1}, "length has leading zero"}, + {[]byte{0x30, 0x81, 0x01}, "end of ber data reached"}, {[]byte{0x30, 0x80, 0x1, 0x2, 0x1, 0x2}, "Invalid BER format"}, - {[]byte{0x30, 0x80, 0x1, 0x2}, "end of ber data reached"}, - {[]byte{0x30, 0x03, 0x01, 0x02}, "length is more than available data"}, + {[]byte{0x30, 0x80, 0x1, 0x2, 0x1}, "BER tag length is more than available data"}, + {[]byte{0x1f, 0x80}, "end of ber data reached"}, + {[]byte{0x30, 0x80}, "end of ber data reached"}, {[]byte{0x30}, "end of ber data reached"}, - {[]byte("0 0 0 0 0 0 0 0\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x020\x02\x1f0"), "end of ber data reached"}, } for _, fixture := range fixtures {