diff options
| -rw-r--r-- | bgpd/ChangeLog | 7 | ||||
| -rw-r--r-- | bgpd/bgp_attr.c | 48 | ||||
| -rw-r--r-- | bgpd/bgp_attr.h | 5 |
3 files changed, 47 insertions, 13 deletions
diff --git a/bgpd/ChangeLog b/bgpd/ChangeLog index 4630ab97..2da28213 100644 --- a/bgpd/ChangeLog +++ b/bgpd/ChangeLog @@ -1,3 +1,10 @@ +2008-06-07 Paul Jakma <paul@jakma.org> + + * bgp_attr.{c,h}: (bgp_mp_{un,}reach_parse) export, for unit tests. + * bgp_attr.c: (bgp_mp_reach_parse) Add logging. Tighten length test + to bounds check against the attribute length rather than the + stream length.. + 2008-06-01 jfletche@gmail.com * bgp_attr.c: (bgp_attr_aspathlimit) fix silly bug in flags check diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 9e7cccaa..19a0869f 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1259,7 +1259,7 @@ bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, } /* Multiprotocol reachability information parse. */ -static int +int bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, struct bgp_nlri *mp_update) { @@ -1277,8 +1277,13 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, /* safe to read statically sized header? */ #define BGP_MP_REACH_MIN_SIZE 5 +#define LEN_LEFT (length - (stream_get_getp(s) - start)) if ((length > STREAM_READABLE(s)) || (length < BGP_MP_REACH_MIN_SIZE)) - return -1; + { + zlog_info ("%s: %s sent invalid length, %lu", + __func__, peer->host, (unsigned long)length); + return -1; + } /* Load AFI, SAFI. */ afi = stream_getw (s); @@ -1287,8 +1292,12 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, /* Get nexthop length. */ attre->mp_nexthop_len = stream_getc (s); - if (STREAM_READABLE(s) < attre->mp_nexthop_len) - return -1; + if (LEN_LEFT < attre->mp_nexthop_len) + { + zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute", + __func__, peer->host, attre->mp_nexthop_len); + return -1; + } /* Nexthop length check. */ switch (attre->mp_nexthop_len) @@ -1330,13 +1339,17 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, break; #endif /* HAVE_IPV6 */ default: - zlog_info ("Wrong multiprotocol next hop length: %d", - attre->mp_nexthop_len); + zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d", + __func__, peer->host, attre->mp_nexthop_len); return -1; } - if (!STREAM_READABLE(s)) - return -1; + if (!LEN_LEFT) + { + zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)", + __func__, peer->host); + return -1; + } { u_char val; @@ -1346,15 +1359,23 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, } /* must have nrli_len, what is left of the attribute */ - nlri_len = length - (stream_get_getp(s) - start); + nlri_len = LEN_LEFT; if ((!nlri_len) || (nlri_len > STREAM_READABLE(s))) - return -1; + { + zlog_info ("%s: (%s) Failed to read NLRI", + __func__, peer->host); + return -1; + } if (safi != BGP_SAFI_VPNV4) { ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), nlri_len); - if (ret < 0) - return -1; + if (ret < 0) + { + zlog_info ("%s: (%s) NLRI doesn't pass sanity check", + __func__, peer->host); + return -1; + } } mp_update->afi = afi; @@ -1365,10 +1386,11 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, stream_forward_getp (s, nlri_len); return 0; +#undef LEN_LEFT } /* Multiprotocol unreachable parse */ -static int +int bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length, struct bgp_nlri *mp_withdraw) { diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index e152b9f4..9647ccf8 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -176,4 +176,9 @@ extern void cluster_unintern (struct cluster_list *); /* Transit attribute prototypes. */ void transit_unintern (struct transit *); +/* Exported for unit-test purposes only */ +extern int bgp_mp_reach_parse (struct peer *, bgp_size_t, struct attr *, + struct bgp_nlri *); +extern int bgp_mp_unreach_parse (struct peer *, bgp_size_t, struct bgp_nlri *); + #endif /* _QUAGGA_BGP_ATTR_H */ |