Hello BIRD maintainers,
I would like to report a possible issue in BIRD.
### Summary
BIRD appears to accept BGP KEEPALIVE messages whose BGP message length is
greater than 19 bytes. RFC 4271 defines KEEPALIVE as only the fixed BGP
message
header, so a valid KEEPALIVE has length 19 and no body.
In dynamic testing, a BGP peer established a session with BIRD and then
sent a
KEEPALIVE with length 23 and a 4-byte payload. BIRD treated the malformed
message as a normal KEEPALIVE, refreshed the session, and kept the BGP state
Established.
I am reporting this as a low-severity RFC conformance / input validation
issue,
not as a high-impact security vulnerability.
### Expected Behavior
When BIRD receives a KEEPALIVE message whose BGP message length is not
exactly
19 bytes, it should reject the message with:
```text
Error Code: Message Header Error
Error Subcode: Bad Message Length
```
That is, an oversized KEEPALIVE should not be accepted as a valid KEEPALIVE.
### Actual Behavior
BIRD accepts a KEEPALIVE with a 4-byte payload:
```text
BGP message type: KEEPALIVE
BGP message length: 23
Payload length: 4 bytes
Payload bytes: 41 42 43 44
```
After the malformed KEEPALIVE is sent, `birdc show protocols all` still
reports
the BGP session as Established:
```text
keepalive_len_test BGP --- up ... Established
BGP state: Established
```
The BIRD log records the malformed message as a normal KEEPALIVE:
```text
keepalive_len_test: Got KEEPALIVE
```
No Header Error / Bad Message Length NOTIFICATION was observed.
### Steps to Reproduce
1. Configure a BGP peer, for example:
```bird
router id 127.0.0.1;
protocol device {
}
protocol bgp keepalive_len_test {
local 127.0.0.1 port 11879 as 65001;
neighbor 127.0.0.2 as 65002;
multihop;
passive on;
ipv4 {
import all;
export none;
};
}
```
2. Establish the BGP session with normal OPEN and KEEPALIVE messages.
3. Send the malformed KEEPALIVE:
```text
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00 17
04
41 42 43 44
```
This is a BGP KEEPALIVE (`type=4`) with total length `0x0017` (23) and a
4-byte body.
### Dynamic Verification
This was reproduced against:
```text
BIRD version 2.19.0+branch.master.880200b1f94c
```
PoC output:
```text
=== BIRD oversized KEEPALIVE length PoC ===
[+] TCP connected: ('127.0.0.2', ...) -> ('127.0.0.1', 11879)
[*] Sent BGP OPEN
[*] Received pre-test message: type=1 body_len=34
[*] Sent normal KEEPALIVE
[*] Received pre-test message: type=4 body_len=0
[*] Sending malformed KEEPALIVE: length=23, payload_len=4
[*] Received post-test message: type=2 body=00000000
...
keepalive_len_test BGP --- up ... Established
[+] Socket still writable after malformed KEEPALIVE
[!] BIRD accepted an oversized KEEPALIVE and kept the session Established
VERDICT: NON_COMPLIANT_REPRODUCED
```
The post-test `type=2 body=00000000` message is an UPDATE carrying an IPv4
End-of-RIB marker. It is not a Header Error NOTIFICATION.
### Source Analysis
The generic receive loop checks only that the BGP message length is within
the
global BGP message bounds:
```c
if ((len < BGP_HEADER_LENGTH) || (len > bgp_max_packet_length(conn)))
{
bgp_error(conn, 1, 2, pkt_start+16, 2);
break;
}
```
Then `bgp_rx_packet()` dispatches KEEPALIVE messages without passing the
packet
length to `bgp_rx_keepalive()`:
```c
case PKT_KEEPALIVE: return bgp_rx_keepalive(conn);
```
`bgp_rx_keepalive()` therefore cannot reject a KEEPALIVE whose length is
greater
than 19 bytes:
```c
static void
bgp_rx_keepalive(struct bgp_conn *conn)
{
...
bgp_start_timer(conn->hold_timer, conn->hold_time);
...
}
```
### Comparison
For comparison, FRR 10.6.1 rejects the same malformed KEEPALIVE with
NOTIFICATION `1/2` (`Message Header Error / Bad Message Length`). FRR has an
explicit check that KEEPALIVE length must equal 19:
```c
type == BGP_MSG_KEEPALIVE && size != BGP_MSG_KEEPALIVE_MIN_SIZE
```
where `BGP_MSG_KEEPALIVE_MIN_SIZE` is `BGP_HEADER_SIZE + 0`.
### Impact
The practical impact appears low. A malformed KEEPALIVE is accepted and
refreshes
the hold timer instead of being rejected as a protocol error.
This does not appear to create parser desynchronization, because the extra
bytes
are consumed as part of the same BGP message. The sender also needs to be an
established BGP peer or an on-path attacker capable of injecting/modifying
BGP
traffic.
The issue is best described as an RFC 4271 conformance and malformed input
validation bug.
### Suggested Fix Direction
Add a KEEPALIVE-specific length check in `bgp_rx_packet()` before
dispatching to
`bgp_rx_keepalive()`. That avoids changing the `bgp_rx_keepalive()` function
signature:
```c
static void
bgp_rx_packet(struct bgp_conn *conn, byte *pkt, uint len)
{
byte type = pkt[18];
if ((type == PKT_KEEPALIVE) && (len != BGP_HEADER_LENGTH))
{
bgp_error(conn, 1, 2, pkt + 16, 2);
return;
}
...
}
```
The `pkt + 16` data field points to the erroneous Length field, matching the
usual Bad Message Length NOTIFICATION data.
Best regards.
