From 26e6fc7e76ba7356d0bfa57daf5c4cf2d80c192f Mon Sep 17 00:00:00 2001 From: Daniel Czerwonk <daniel@dan-nrw.de> Date: Sat, 21 Jul 2018 10:57:31 +0200 Subject: [PATCH] fix for #102 #103 #104 --- protocols/bgp/packet/mp_reach_nlri.go | 21 +++++++--- protocols/bgp/packet/mp_unreach_nlri.go | 19 +++++---- protocols/bgp/packet/path_attributes_test.go | 41 ++++++++++++++++++++ 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/protocols/bgp/packet/mp_reach_nlri.go b/protocols/bgp/packet/mp_reach_nlri.go index 1aa9e92a..65665f29 100644 --- a/protocols/bgp/packet/mp_reach_nlri.go +++ b/protocols/bgp/packet/mp_reach_nlri.go @@ -38,8 +38,13 @@ func (n *MultiProtocolReachNLRI) serialize(buf *bytes.Buffer) uint16 { func deserializeMultiProtocolReachNLRI(b []byte) (MultiProtocolReachNLRI, error) { n := MultiProtocolReachNLRI{} nextHopLength := uint8(0) - variable := make([]byte, len(b)-4) + variableLength := len(b) - 4 // 4 <- AFI + SAFI + NextHopLength + if variableLength <= 0 { + return n, fmt.Errorf("Invalid length of MP_REACH_NLRI: expected more than 4 bytes but got %d", len(b)) + } + + variable := make([]byte, variableLength) fields := []interface{}{ &n.AFI, &n.SAFI, @@ -51,19 +56,25 @@ func deserializeMultiProtocolReachNLRI(b []byte) (MultiProtocolReachNLRI, error) return MultiProtocolReachNLRI{}, err } + budget := variableLength + if budget < int(nextHopLength) { + return MultiProtocolReachNLRI{}, + fmt.Errorf("Failed to decode next hop IP: expected %d bytes for NLRI, only %d remaining", nextHopLength, budget) + } + n.NextHop, err = bnet.IPFromBytes(variable[:nextHopLength]) if err != nil { return MultiProtocolReachNLRI{}, fmt.Errorf("Failed to decode next hop IP: %v", err) } - - variable = variable[1+nextHopLength:] + budget -= int(nextHopLength) n.Prefixes = make([]bnet.Prefix, 0) - - if len(variable) == 0 { + if budget == 0 { return n, nil } + variable = variable[1+nextHopLength:] // 1 <- RESERVED field + idx := uint16(0) for idx < uint16(len(variable)) { pfxLen := variable[idx] diff --git a/protocols/bgp/packet/mp_unreach_nlri.go b/protocols/bgp/packet/mp_unreach_nlri.go index 3a78b00f..63cccd58 100644 --- a/protocols/bgp/packet/mp_unreach_nlri.go +++ b/protocols/bgp/packet/mp_unreach_nlri.go @@ -30,36 +30,41 @@ func (n *MultiProtocolUnreachNLRI) serialize(buf *bytes.Buffer) uint16 { func deserializeMultiProtocolUnreachNLRI(b []byte) (MultiProtocolUnreachNLRI, error) { n := MultiProtocolUnreachNLRI{} - prefix := make([]byte, len(b)-3) + prefixesLength := len(b) - 3 // 3 <- AFI + SAFI + if prefixesLength < 0 { + return n, fmt.Errorf("Invalid length of MP_UNREACH_NLRI: expected more than 3 bytes but got %d", len(b)) + } + + prefixes := make([]byte, prefixesLength) fields := []interface{}{ &n.AFI, &n.SAFI, - &prefix, + &prefixes, } err := decode(bytes.NewBuffer(b), fields) if err != nil { return MultiProtocolUnreachNLRI{}, err } - if len(prefix) == 0 { + if len(prefixes) == 0 { return n, nil } idx := uint16(0) - for idx < uint16(len(prefix)) { - pfxLen := prefix[idx] + for idx < uint16(len(prefixes)) { + pfxLen := prefixes[idx] numBytes := uint16(BytesInAddr(pfxLen)) idx++ - r := uint16(len(prefix)) - idx + r := uint16(len(prefixes)) - idx if r < numBytes { return MultiProtocolUnreachNLRI{}, fmt.Errorf("expected %d bytes for NLRI, only %d remaining", numBytes, r) } start := idx end := idx + numBytes - pfx, err := deserializePrefix(prefix[start:end], pfxLen, n.AFI) + pfx, err := deserializePrefix(prefixes[start:end], pfxLen, n.AFI) if err != nil { return MultiProtocolUnreachNLRI{}, err } diff --git a/protocols/bgp/packet/path_attributes_test.go b/protocols/bgp/packet/path_attributes_test.go index dd4b31e8..606ee24c 100644 --- a/protocols/bgp/packet/path_attributes_test.go +++ b/protocols/bgp/packet/path_attributes_test.go @@ -934,6 +934,40 @@ func TestDecodeMultiProtocolReachNLRI(t *testing.T) { }, }, }, + { + name: "MP_REACH_NLRI with invalid length", + input: []byte{ + 0x00, 0x02, // AFI + }, + wantFail: true, + }, + { + name: "MP_REACH_NLRI with invalid length 2", + input: []byte{ + 0x00, 0x02, // AFI + 0x01, // SAFI + 0x10, 0x20, 0x01, 0x06, 0x78, 0x01, 0xe0, 0x00, // incomplete NextHop + }, + wantFail: true, + }, + { + name: "MP_REACH_NLRI without prefixes", + input: []byte{ + 0x00, 0x02, // AFI + 0x01, // SAFI + 0x10, 0x20, 0x01, 0x06, 0x78, 0x01, 0xe0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, // NextHop + 0x00, // RESERVED + }, + expected: &PathAttribute{ + Length: 21, + Value: MultiProtocolReachNLRI{ + AFI: IPv6AFI, + SAFI: UnicastSAFI, + NextHop: bnet.IPv6FromBlocks(0x2001, 0x678, 0x1e0, 0, 0, 0, 0, 0x2), + Prefixes: []bnet.Prefix{}, + }, + }, + }, } t.Parallel() @@ -998,6 +1032,13 @@ func TestDecodeMultiProtocolUnreachNLRI(t *testing.T) { }, }, }, + { + name: "MP_UNREACH_NLRI with invalid length", + input: []byte{ + 0x00, 0x02, // AFI + }, + wantFail: true, + }, } t.Parallel() -- GitLab