summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDonatas Abraitis <donatas@opensourcerouting.org>2024-06-11 09:03:17 +0200
committerDonatas Abraitis <donatas@opensourcerouting.org>2024-06-11 09:03:17 +0200
commite7bc47b5013758e1d99d114f6310746c1cc488ca (patch)
treec1343b7d4d7d16b4eb01489d934213323ed2a562
parentMerge pull request #16183 from LabNConsulting/chopps/notif-doc-update (diff)
downloadfrr-e7bc47b5013758e1d99d114f6310746c1cc488ca.tar.xz
frr-e7bc47b5013758e1d99d114f6310746c1cc488ca.zip
bgpd: Check against extended community unit size for link bandwidth
If we receive a malformed packets, this could lead ptr_get_be64() reading the packets more than needed (heap overflow). ``` Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1". 0 0xaaaaaadf86ec in __asan_memcpy (/home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/.libs/bgpd+0x3586ec) (BuildId: 78123cd26ada92b8b59fc0d74d292ba70c9d2e01) 1 0xaaaaaaeb60fc in ptr_get_be64 /home/ubuntu/frr-public/frr_public_private-libfuzzer/./lib/stream.h:377:2 2 0xaaaaaaeb5b90 in ecommunity_linkbw_present /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_ecommunity.c:1895:10 3 0xaaaaaae50f30 in bgp_attr_ext_communities /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_attr.c:2639:8 4 0xaaaaaae49d58 in bgp_attr_parse /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_attr.c:3776:10 5 0xaaaaab063260 in bgp_update_receive /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:2371:20 6 0xaaaaab05df00 in bgp_process_packet /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:4063:11 7 0xaaaaaae36110 in LLVMFuzzerTestOneInput /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_main.c:582:3 ``` This is triggered when receiving such a packet (malformed): ``` (gdb) bt 0 ecommunity_linkbw_present (ecom=0x555556287990, bw=bw@entry=0x7fffffffda68) at bgpd/bgp_ecommunity.c:1802 1 0x000055555564fcac in bgp_attr_ext_communities (args=0x7fffffffd840) at bgpd/bgp_attr.c:2619 2 bgp_attr_parse (peer=peer@entry=0x55555628cdf0, attr=attr@entry=0x7fffffffd960, size=size@entry=20, mp_update=mp_update@entry=0x7fffffffd940, mp_withdraw=mp_withdraw@entry=0x7fffffffd950) at bgpd/bgp_attr.c:3755 3 0x00005555556aa655 in bgp_update_receive (connection=connection@entry=0x5555562aa030, peer=peer@entry=0x55555628cdf0, size=size@entry=41) at bgpd/bgp_packet.c:2324 4 0x00005555556afab7 in bgp_process_packet (thread=<optimized out>) at bgpd/bgp_packet.c:3897 5 0x00007ffff7ac2f73 in event_call (thread=thread@entry=0x7fffffffdc70) at lib/event.c:2011 6 0x00007ffff7a6fb90 in frr_run (master=0x555555bc7c90) at lib/libfrr.c:1212 7 0x00005555556457e1 in main (argc=<optimized out>, argv=<optimized out>) at bgpd/bgp_main.c:543 (gdb) p *ecom $1 = {refcnt = 1, unit_size = 8 '\b', disable_ieee_floating = false, size = 2, val = 0x555556282150 "", str = 0x5555562a9c30 "UNK:0, 255 UNK:2, 6"} ``` Reported-by: Iggy Frankovic <iggyfran@amazon.com> Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
-rw-r--r--bgpd/bgp_ecommunity.c20
1 files changed, 16 insertions, 4 deletions
diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c
index 66898d07b..1beb0307d 100644
--- a/bgpd/bgp_ecommunity.c
+++ b/bgpd/bgp_ecommunity.c
@@ -1856,7 +1856,7 @@ ecommunity_add_origin_validation_state(enum rpki_states rpki_state,
*/
const uint8_t *ecommunity_linkbw_present(struct ecommunity *ecom, uint64_t *bw)
{
- const uint8_t *eval;
+ const uint8_t *data;
uint32_t i;
if (bw)
@@ -1869,10 +1869,19 @@ const uint8_t *ecommunity_linkbw_present(struct ecommunity *ecom, uint64_t *bw)
const uint8_t *pnt;
uint8_t type, sub_type;
- eval = pnt = (ecom->val + (i * ecom->unit_size));
+ data = pnt = (ecom->val + (i * ecom->unit_size));
type = *pnt++;
sub_type = *pnt++;
+ const uint8_t *end = data + ecom->unit_size;
+ size_t len = end - data;
+
+ /* Sanity check for extended communities lenght, to avoid
+ * overrun when dealing with bits, e.g. ptr_get_be64().
+ */
+ if (len < ecom->unit_size)
+ return NULL;
+
if ((type == ECOMMUNITY_ENCODE_AS ||
type == ECOMMUNITY_ENCODE_AS_NON_TRANS) &&
sub_type == ECOMMUNITY_LINK_BANDWIDTH) {
@@ -1886,11 +1895,14 @@ const uint8_t *ecommunity_linkbw_present(struct ecommunity *ecom, uint64_t *bw)
? bwval
: ieee_float_uint32_to_uint32(
bwval));
- return eval;
+ return data;
} else if (type == ECOMMUNITY_ENCODE_AS4 &&
sub_type == ECOMMUNITY_EXTENDED_LINK_BANDWIDTH) {
uint64_t bwval;
+ if (len < IPV6_ECOMMUNITY_SIZE)
+ return NULL;
+
pnt += 2; /* Reserved */
pnt = ptr_get_be64(pnt, &bwval);
(void)pnt;
@@ -1898,7 +1910,7 @@ const uint8_t *ecommunity_linkbw_present(struct ecommunity *ecom, uint64_t *bw)
if (bw)
*bw = bwval;
- return eval;
+ return data;
}
}