Skip to content

Commit

Permalink
bgpd: Check if we have really enough data before doing memcpy for sof…
Browse files Browse the repository at this point in the history
…tware version

If we receive CAPABILITY message (software-version), we SHOULD check if we really
have enough data before doing memcpy(), that could also lead to buffer overflow.

(data + len > end) is not enough, because after this check we do data++ and later
memcpy(..., data, len). That means we have one more byte.

Hit this through fuzzing by

```
    0 0xaaaaaadf872c in __asan_memcpy (/home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/.libs/bgpd+0x35872c) (BuildId: 9c6e455d0d9a20f5a4d2f035b443f50add9564d7)
    1 0xaaaaab06bfbc in bgp_dynamic_capability_software_version /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3713:3
    2 0xaaaaab05ccb4 in bgp_capability_msg_parse /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3839:4
    3 0xaaaaab05c074 in bgp_capability_receive /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3980:9
    4 0xaaaaab05e48c in bgp_process_packet /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:4109:11
    5 0xaaaaaae36150 in LLVMFuzzerTestOneInput /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_main.c:582:3
```

Hit this again by Iggy \m/

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
  • Loading branch information
ton31337 committed Jun 12, 2024
1 parent d5b0c76 commit 5d7af51
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion bgpd/bgp_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -3694,7 +3694,7 @@ static void bgp_dynamic_capability_software_version(uint8_t *pnt, int action,
char soft_version[BGP_MAX_SOFT_VERSION + 1] = {};

if (action == CAPABILITY_ACTION_SET) {
if (data + len > end) {
if (data + len + 1 > end) {
zlog_err("%pBP: Received invalid Software Version capability length %d",
peer, len);
return;
Expand Down

0 comments on commit 5d7af51

Please sign in to comment.