Merge pull request #105 from bio-routing/fix/multi_protocol_decode_robustness

Increased Multi Protocol decoding robustness

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]

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
 		}

protocols/bgp/packet/path_attributes_test.go

@@ -934,6 +934,51 @@ 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{},
+				},
+			},
+		},
+		{
+			name: "MP_REACH_NLRI with invalid 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
+				0x30, 0x26, 0x00, // Prefix
+			},
+			wantFail: true,
+		},
 	}
 
 	t.Parallel()
@@ -998,6 +1043,22 @@ func TestDecodeMultiProtocolUnreachNLRI(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "MP_UNREACH_NLRI with invalid length",
+			input: []byte{
+				0x00, 0x02, // AFI
+			},
+			wantFail: true,
+		},
+		{
+			name: "MP_UNREACH_NLRI with invalid prefixes",
+			input: []byte{
+				0x00, 0x02, // AFI
+				0x01,                   // SAFI
+				0x2c, 0x26, 0x20, 0x01, // Prefix
+			},
+			wantFail: true,
+		},
 	}
 
 	t.Parallel()