Multiple heap OOB reads in attr_init() and other paths — no bounds validation on declared lengths

[afldl]

I found several heap out-of-bounds reads in libbgpdump while doing differential fuzzing against FRRouting and GoBGP. They all share the same root cause: the parser trusts length fields in BGP UPDATE messages without checking whether enough bytes are actually available.

I'm filing these as one issue since the underlying pattern is the same. Tested on 1.6.2 and current master.

---

1. attr_init() — Total Path Attribute Length not bounds-checked (CWE-125)

The main one. process_attributes() reads the 16-bit Total Path Attribute Length from a BGP UPDATE and passes it straight to attr_init(), which does malloc(len) + memcpy(..., len). If the declared length is larger than what's actually in the stream, you get a heap OOB read.

// bgpdump_lib.c process_attributes() ~line 1224
int total = mstream_getw(s, NULL);          /* reads declared length */
attributes_t *attr = attr_init(s, total);   /* no bounds check */

// attr_init() ~line 1019
attr->data = malloc(len);
memcpy(attr->data, &s->start[s->position], len);  /* OOB when len >> available */

There IS a check after the copy (mstream_can_read(&copy) != total) but it only warns — the OOB read already happened.

PoC: BGP UPDATE with Total PA Length=4096 but only ~16 bytes of attribute data, wrapped in MRT BGP4MP. ASAN reports heap-buffer-overflow in __asan_memcpy at attr_init:1032.

FRRouting and GoBGP both reject the same input.

2. AS4_PATH short length → OOB in check_new_aspath() (CWE-125)

When AS4_PATH attribute has length < 2, create_aspath() allocates a tiny buffer, then check_new_aspath() iterates over it with struct assegment pointers. Since assegment needs at least 2 bytes (type + len), accessing segment->length reads past the buffer.

3. MP_REACH_NLRI zero length → OOB in process_mp_announce() (CWE-125)

process_mp_announce() reads from the stream without checking remaining bytes when MP_REACH_NLRI has zero-length body.

4. read_prefix_list() — prefix_len > 32 → heap overflow (CWE-122)

read_prefix_list() doesn't validate p_len. When prefix_len=220, p_bytes = (220+7)/8 = 28 overflows the 16-byte BGPDUMP_IP_ADDRESS buffer.

5. attr_init() called from TABLE_DUMP path (CWE-125)

Same as #1 but triggered through process_mrtd_table_dump() (MRT type 12) — total_attr_len from TABLE_DUMP headers also feeds into attr_init() unchecked.

---

Fix for #1 (the most critical):

Validate remaining stream bytes before copying:

 int total = mstream_getw(s, NULL);
+
+if (total < 0 || total > mstream_can_read(s)) {
+    warn("malformed UPDATE: Total Path Attribute Length %d exceeds available %u",
+         total, mstream_can_read(s));
+    return NULL;
+}
+
 attributes_t *attr = attr_init(s, total);

Similar bounds checks needed for paths #2-#5.

Tested against: FRRouting (rejects all malformed inputs), GoBGP (rejects all malformed inputs). bgpdump is the only implementation that proceeds with the OOB reads.

Found through differential fuzzing (PathDiff v23).

[spakka]

Hi,

Thanks for the report. I'll review this when I get some cycles to work on it soon!

Regards,
Colin