From 0558066261625d0f4e7d5c118a0efd4238d24715 Mon Sep 17 00:00:00 2001 From: Sergey Elantsev Date: Sun, 20 Dec 2020 18:13:10 +0300 Subject: fixed possible crashes on parsing of bgp messages --- pkg/packet/bgp/bgp.go | 8 ++- pkg/packet/bgp/bgp_test.go | 67 +++++++++++++++++++++ pkg/packet/bgp/prefix_sid.go | 2 +- pkg/packet/bgp/sr_policy.go | 5 +- .../000852858287eb2ece5cd12aabef453f87b77adb | Bin 0 -> 64 bytes .../0b229debaa2fc1e8b997c9894ad3184cce8c7bb4 | Bin 0 -> 64 bytes .../1d1fb484828b4f23af3ff0ab71b67682d5876fd6 | Bin 0 -> 64 bytes .../3765bb2392d48ca8edaf0b5e39209eb2d1b87c53 | Bin 0 -> 64 bytes .../4d4dc736bc89bf41363774d282b8a7641c3e7a98 | Bin 0 -> 31 bytes 9 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 pkg/packet/bgp/testdata/bad-len/000852858287eb2ece5cd12aabef453f87b77adb create mode 100644 pkg/packet/bgp/testdata/bad-len/0b229debaa2fc1e8b997c9894ad3184cce8c7bb4 create mode 100644 pkg/packet/bgp/testdata/bad-len/1d1fb484828b4f23af3ff0ab71b67682d5876fd6 create mode 100644 pkg/packet/bgp/testdata/bad-len/3765bb2392d48ca8edaf0b5e39209eb2d1b87c53 create mode 100644 pkg/packet/bgp/testdata/bad-len/4d4dc736bc89bf41363774d282b8a7641c3e7a98 diff --git a/pkg/packet/bgp/bgp.go b/pkg/packet/bgp/bgp.go index 2f117830..150c6c24 100644 --- a/pkg/packet/bgp/bgp.go +++ b/pkg/packet/bgp/bgp.go @@ -977,6 +977,9 @@ func (o *OptionParameterCapability) DecodeFromBytes(data []byte) error { return err } o.Capability = append(o.Capability, c) + if c.Len() == 0 || len(data) < c.Len() { + return NewMessageError(BGP_ERROR_MESSAGE_HEADER_ERROR, BGP_ERROR_SUB_BAD_MESSAGE_LENGTH, nil, "Bad capability length") + } data = data[c.Len():] } return nil @@ -1051,7 +1054,7 @@ func (msg *BGPOpen) DecodeFromBytes(data []byte, options ...*MarshallingOption) } paramtype := data[0] paramlen := data[1] - if rest < paramlen+2 { + if paramlen >= 254 || rest < paramlen+2 { return NewMessageError(BGP_ERROR_MESSAGE_HEADER_ERROR, BGP_ERROR_SUB_BAD_MESSAGE_LENGTH, nil, "Malformed BGP Open message") } rest -= paramlen + 2 @@ -12170,6 +12173,9 @@ func (p *PathAttributeAigp) DecodeFromBytes(data []byte, options ...*Marshalling for len(value) > 3 { typ := value[0] length := binary.BigEndian.Uint16(value[1:3]) + if length <= 3 { + return NewMessageError(BGP_ERROR_MESSAGE_HEADER_ERROR, BGP_ERROR_SUB_BAD_MESSAGE_LENGTH, nil, "Malformed BGP message") + } if len(value) < int(length) { break } diff --git a/pkg/packet/bgp/bgp_test.go b/pkg/packet/bgp/bgp_test.go index d821d072..112a9b4f 100644 --- a/pkg/packet/bgp/bgp_test.go +++ b/pkg/packet/bgp/bgp_test.go @@ -18,9 +18,13 @@ package bgp import ( "bytes" "encoding/binary" + "io/ioutil" "net" + "os" + "path/filepath" "reflect" "strconv" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -1194,6 +1198,69 @@ func TestFuzzCrashers(t *testing.T) { } } +func TestParseMessageWithBadLength(t *testing.T) { + type testCase struct { + fname string + data []byte + } + + var cases []testCase + root := filepath.Join("testdata", "bad-len") + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + if path == root { + return nil + } + return filepath.SkipDir + } + fname := filepath.Base(path) + if strings.ContainsRune(fname, '.') { + return nil + } + data, err := ioutil.ReadFile(path) + if err != nil { + return err + } + cases = append(cases, testCase{ + fname: fname, + data: data, + }) + return nil + }) + if err != nil { + t.Fatal(err) + } + + for _, tt := range cases { + t.Run(tt.fname, func(t *testing.T) { + msg, err := ParseBGPMessage(tt.data) + if err == nil { + _, err = msg.Serialize() + if err != nil { + t.Fatal("failed to serialize:", err) + } + return + } + + switch e := err.(type) { + case *MessageError: + switch e.TypeCode { + case BGP_ERROR_MESSAGE_HEADER_ERROR: + if e.SubTypeCode != BGP_ERROR_SUB_BAD_MESSAGE_LENGTH { + t.Fatalf("got unexpected message type and data: %v", e) + } + } + default: + t.Fatalf("got unwxpected error type %T: %v", err, err) + } + + }) + } +} + func TestNormalizeFlowSpecOpValues(t *testing.T) { tests := []struct { msg string diff --git a/pkg/packet/bgp/prefix_sid.go b/pkg/packet/bgp/prefix_sid.go index a7983bcd..32f575ee 100644 --- a/pkg/packet/bgp/prefix_sid.go +++ b/pkg/packet/bgp/prefix_sid.go @@ -52,7 +52,7 @@ func (s *TLV) DecodeFromBytes(data []byte) ([]byte, error) { p++ s.Length = binary.BigEndian.Uint16(data[p : p+2]) - if len(data) < s.Len() { + if s.Len() < prefixSIDtlvHdrLen || len(data) < s.Len() { return nil, malformedAttrListErr("decoding failed: Prefix SID TLV malformed") } diff --git a/pkg/packet/bgp/sr_policy.go b/pkg/packet/bgp/sr_policy.go index e7baa069..cdd89db2 100644 --- a/pkg/packet/bgp/sr_policy.go +++ b/pkg/packet/bgp/sr_policy.go @@ -742,9 +742,12 @@ func (t *TunnelEncapSubTLVSRSegmentList) DecodeFromBytes(data []byte) error { if err != nil { return NewMessageError(BGP_ERROR_UPDATE_MESSAGE_ERROR, BGP_ERROR_SUB_MALFORMED_ATTRIBUTE_LIST, nil, err.Error()) } + if len(value) < 1 { + return NewMessageError(BGP_ERROR_MESSAGE_HEADER_ERROR, BGP_ERROR_SUB_BAD_MESSAGE_LENGTH, nil, "Malformed BGP message") + } // Skip reserved byte to access inner SubTLV type value = value[1:] - segments := make([]TunnelEncapSubTLVInterface, 0) + var segments []TunnelEncapSubTLVInterface p := 0 for p < t.TunnelEncapSubTLV.Len()-4 { var segment TunnelEncapSubTLVInterface diff --git a/pkg/packet/bgp/testdata/bad-len/000852858287eb2ece5cd12aabef453f87b77adb b/pkg/packet/bgp/testdata/bad-len/000852858287eb2ece5cd12aabef453f87b77adb new file mode 100644 index 00000000..9fed8106 Binary files /dev/null and b/pkg/packet/bgp/testdata/bad-len/000852858287eb2ece5cd12aabef453f87b77adb differ diff --git a/pkg/packet/bgp/testdata/bad-len/0b229debaa2fc1e8b997c9894ad3184cce8c7bb4 b/pkg/packet/bgp/testdata/bad-len/0b229debaa2fc1e8b997c9894ad3184cce8c7bb4 new file mode 100644 index 00000000..468f2e89 Binary files /dev/null and b/pkg/packet/bgp/testdata/bad-len/0b229debaa2fc1e8b997c9894ad3184cce8c7bb4 differ diff --git a/pkg/packet/bgp/testdata/bad-len/1d1fb484828b4f23af3ff0ab71b67682d5876fd6 b/pkg/packet/bgp/testdata/bad-len/1d1fb484828b4f23af3ff0ab71b67682d5876fd6 new file mode 100644 index 00000000..416d4be8 Binary files /dev/null and b/pkg/packet/bgp/testdata/bad-len/1d1fb484828b4f23af3ff0ab71b67682d5876fd6 differ diff --git a/pkg/packet/bgp/testdata/bad-len/3765bb2392d48ca8edaf0b5e39209eb2d1b87c53 b/pkg/packet/bgp/testdata/bad-len/3765bb2392d48ca8edaf0b5e39209eb2d1b87c53 new file mode 100644 index 00000000..68afda16 Binary files /dev/null and b/pkg/packet/bgp/testdata/bad-len/3765bb2392d48ca8edaf0b5e39209eb2d1b87c53 differ diff --git a/pkg/packet/bgp/testdata/bad-len/4d4dc736bc89bf41363774d282b8a7641c3e7a98 b/pkg/packet/bgp/testdata/bad-len/4d4dc736bc89bf41363774d282b8a7641c3e7a98 new file mode 100644 index 00000000..4beb9d2c Binary files /dev/null and b/pkg/packet/bgp/testdata/bad-len/4d4dc736bc89bf41363774d282b8a7641c3e7a98 differ -- cgit v1.2.3