Skip to content

Commit 08ebbd8

Browse files
committed
bgpd: Use stream_new_expandable() for BMP code to avoid overflow
Also, validate and drop packets later exceeding 65k. Reported-by: Qifan Zhang, Palo Alto Networks Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
1 parent 499504b commit 08ebbd8

1 file changed

Lines changed: 27 additions & 4 deletions

File tree

bgpd/bgp_bmp.c

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,15 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
11061106

11071107
bpacket_attr_vec_arr_reset(&vecarr);
11081108

1109-
s = stream_new(BGP_MAX_PACKET_SIZE);
1109+
/*
1110+
* Use an expandable stream so that an oversized re-encoded UPDATE
1111+
* (large unknown-transitive attribute plus route-map additions) does
1112+
* not silently truncate via CHECK_SIZE or assert in stream_put_prefix.
1113+
* The on-wire UPDATE inside a BMP Route-Monitoring message must still
1114+
* fit BGP's 16-bit length field, so we drop the route if the encoded
1115+
* size exceeds BGP_MAX_PACKET_SIZE.
1116+
*/
1117+
s = stream_new_expandable(BGP_MAX_PACKET_SIZE);
11101118
bgp_packet_set_marker(s, BGP_MSG_UPDATE);
11111119

11121120
/* 2: withdrawn routes length */
@@ -1120,8 +1128,6 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
11201128
total_attr_len = bgp_packet_attribute(NULL, peer, s, attr, &vecarr, NULL, afi, safi, peer,
11211129
NULL, NULL, 0, 0, 0, 0, NULL, NULL);
11221130

1123-
/* space check? */
1124-
11251131
/* peer_cap_enhe & add-path removed */
11261132
if (afi == AFI_IP && safi == SAFI_UNICAST)
11271133
stream_put_prefix(s, p);
@@ -1137,6 +1143,13 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
11371143
total_attr_len += stream_get_endp(s) - p1;
11381144
}
11391145

1146+
if (stream_get_endp(s) > BGP_MAX_PACKET_SIZE) {
1147+
zlog_warn("bmp: skipping route-monitoring for %pFX: re-encoded UPDATE exceeds %u bytes",
1148+
p, BGP_MAX_PACKET_SIZE);
1149+
stream_free(s);
1150+
return NULL;
1151+
}
1152+
11401153
/* set the total attribute length correctly */
11411154
stream_putw_at(s, attrlen_pos, total_attr_len);
11421155
bgp_packet_set_size(s);
@@ -1152,7 +1165,7 @@ static struct stream *bmp_withdraw(const struct prefix *p,
11521165
bgp_size_t total_attr_len = 0;
11531166
bgp_size_t unfeasible_len;
11541167

1155-
s = stream_new(BGP_MAX_PACKET_SIZE);
1168+
s = stream_new_expandable(BGP_MAX_PACKET_SIZE);
11561169

11571170
bgp_packet_set_marker(s, BGP_MSG_UPDATE);
11581171
stream_putw(s, 0);
@@ -1179,6 +1192,13 @@ static struct stream *bmp_withdraw(const struct prefix *p,
11791192
stream_putw_at(s, attrlen_pos, total_attr_len);
11801193
}
11811194

1195+
if (stream_get_endp(s) > BGP_MAX_PACKET_SIZE) {
1196+
zlog_warn("bmp: skipping route-monitoring withdraw for %pFX: encoded UPDATE exceeds %u bytes",
1197+
p, BGP_MAX_PACKET_SIZE);
1198+
stream_free(s);
1199+
return NULL;
1200+
}
1201+
11821202
bgp_packet_set_size(s);
11831203
return s;
11841204
}
@@ -1208,6 +1228,9 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags,
12081228
else
12091229
msg = bmp_withdraw(p, prd, afi, safi);
12101230

1231+
if (!msg)
1232+
return;
1233+
12111234
hdr = stream_new(BGP_MAX_PACKET_SIZE);
12121235
bmp_common_hdr(hdr, BMP_VERSION_3, BMP_TYPE_ROUTE_MONITORING);
12131236
bmp_per_peer_hdr(hdr, peer->bgp, peer, flags, peer_type_flag, peer_distinguisher,

0 commit comments

Comments
 (0)