BGP 4-byte ASN bug fixes

2009-05-07 / 2009-05-08
Credit: Chris Caputo
Risk: Medium
Local: No
Remote: Yes
CWE: CWE-Other


CVSS Base Score: 5/10
Impact Subscore: 2.9/10
Exploitability Subscore: 10/10
Exploit range: Remote
Attack complexity: Low
Authentication: No required
Confidentiality impact: None
Integrity impact: None
Availability impact: Partial

* bgpd/bgp_aspath.c: (aspath_make_str_count) "assert (len < str_size)" was getting hit under certain 4-byte ASN conditions. New realloc strategy. * bgpd/bgp_aspath.c: (aspath_key_make) const warning fix. "%d" -> "%u" 4-byte ASN corrections. Prevent negative number when ASN is above 2^31.: bgpd/bgp_attr.c bgpd/bgp_community.c bgpd/bgp_debug.c bgpd/bgp_ecommunity.c bgpd/bgp_mplsvpn.c bgpd/bgp_packet.c bgpd/bgp_route.c bgpd/bgp_vty.c bgpd/bgpd.c --- bgpd/bgp_aspath.c | 85 ++++++++++++++++++------------------------------- bgpd/bgp_attr.c | 2 +- bgpd/bgp_community.c | 2 +- bgpd/bgp_debug.c | 2 +- bgpd/bgp_ecommunity.c | 4 +- bgpd/bgp_mplsvpn.c | 6 ++-- bgpd/bgp_packet.c | 8 ++-- bgpd/bgp_route.c | 8 ++-- bgpd/bgp_vty.c | 20 ++++++------ bgpd/bgpd.c | 10 +++--- 10 files changed, 62 insertions(+), 85 deletions(-) diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index 006fc91..a1e4608 100644 --- a/bgpd/bgp_aspath.c +++ b/bgpd/bgp_aspath.c @@ -393,25 +393,6 @@ aspath_delimiter_char (u_char type, u_char which) return ' '; } -/* countup asns from this segment and index onward */ -static int -assegment_count_asns (struct assegment *seg, int from) -{ - int count = 0; - while (seg) - { - if (!from) - count += seg->length; - else - { - count += (seg->length - from); - from = 0; - } - seg = seg->next; - } - return count; -} - unsigned int aspath_count_confeds (struct aspath *aspath) { @@ -521,6 +502,21 @@ aspath_count_numas (struct aspath *aspath) return num; } +static void +aspath_make_str_big_enough (int len, + char **str_buf, + int *str_size, + int count_to_be_added) +{ +#define TERMINATOR 1 + while (len + count_to_be_added + TERMINATOR > *str_size) + { + *str_size *= 2; + *str_buf = XREALLOC (MTYPE_AS_STR, *str_buf, *str_size); + } +#undef TERMINATOR +} + /* Convert aspath structure to string expression. */ static char * aspath_make_str_count (struct aspath *as) @@ -540,18 +536,7 @@ aspath_make_str_count (struct aspath *as) seg = as->segments; - /* ASN takes 5 chars at least, plus seperator, see below. - * If there is one differing segment type, we need an additional - * 2 chars for segment delimiters, and the final '\0'. - * Hopefully this is large enough to avoid hitting the realloc - * code below for most common sequences. - * - * With 32bit ASNs, this range will increase, but only worth changing - * once there are significant numbers of ASN >= 100000 - */ -#define ASN_STR_LEN (5 + 1) - str_size = MAX (assegment_count_asns (seg, 0) * ASN_STR_LEN + 2 + 1, - ASPATH_STR_DEFAULT_LEN); + str_size = ASPATH_STR_DEFAULT_LEN; str_buf = XMALLOC (MTYPE_AS_STR, str_size); while (seg) @@ -575,32 +560,24 @@ aspath_make_str_count (struct aspath *as) return NULL; } - /* We might need to increase str_buf, particularly if path has - * differing segments types, our initial guesstimate above will - * have been wrong. need 5 chars for ASN, a seperator each and - * potentially two segment delimiters, plus a space between each - * segment and trailing zero. - * - * This may need to revised if/when significant numbers of - * ASNs >= 100000 are assigned and in-use on the internet... - */ -#define SEGMENT_STR_LEN(X) (((X)->length * ASN_STR_LEN) + 2 + 1 + 1) - if ( (len + SEGMENT_STR_LEN(seg)) > str_size) - { - str_size = len + SEGMENT_STR_LEN(seg); - str_buf = XREALLOC (MTYPE_AS_STR, str_buf, str_size); - } -#undef ASN_STR_LEN -#undef SEGMENT_STR_LEN - if (seg->type != AS_SEQUENCE) - len += snprintf (str_buf + len, str_size - len, - "%c", - aspath_delimiter_char (seg->type, AS_SEG_START)); + { + aspath_make_str_big_enough (len, &str_buf, &str_size, 1); /* %c */ + len += snprintf (str_buf + len, str_size - len, + "%c", + aspath_delimiter_char (seg->type, AS_SEG_START)); + } /* write out the ASNs, with their seperators, bar the last one*/ for (i = 0; i < seg->length; i++) { +#define APPROX_DIG_CNT(x) (x < 100000U ? 5 : 10) + /* %u + %c + %c + " " (last two are below loop) */ + aspath_make_str_big_enough (len, + &str_buf, + &str_size, + APPROX_DIG_CNT(seg->as[i]) + 1 + 1 + 1); + len += snprintf (str_buf + len, str_size - len, "%u", seg->as[i]); if (i < (seg->length - 1)) @@ -1771,8 +1748,8 @@ aspath_key_make (void *p) static int aspath_cmp (const void *arg1, const void *arg2) { - const struct assegment *seg1 = ((struct aspath *)arg1)->segments; - const struct assegment *seg2 = ((struct aspath *)arg2)->segments; + const struct assegment *seg1 = ((const struct aspath *)arg1)->segments; + const struct assegment *seg2 = ((const struct aspath *)arg2)->segments; while (seg1 || seg2) { diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index d116c30..f38db41 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -857,7 +857,7 @@ static int bgp_attr_aspath_check( struct peer *peer, && ! aspath_firstas_check (attr->aspath, peer->as)) { zlog (peer->log, LOG_ERR, - "%s incorrect first AS (must be %d)", peer->host, peer->as); + "%s incorrect first AS (must be %u)", peer->host, peer->as); bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_AS_PATH); diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c index 1cafdb3..a05ea6c 100644 --- a/bgpd/bgp_community.c +++ b/bgpd/bgp_community.c @@ -282,7 +282,7 @@ community_com2str (struct community *com) default: as = (comval >> 16) & 0xFFFF; val = comval & 0xFFFF; - sprintf (pnt, "%d:%d", as, val); + sprintf (pnt, "%u:%d", as, val); pnt += strlen (pnt); break; } diff --git a/bgpd/bgp_debug.c b/bgpd/bgp_debug.c index 757b9cf..1d5bf6b 100644 --- a/bgpd/bgp_debug.c +++ b/bgpd/bgp_debug.c @@ -205,7 +205,7 @@ bgp_dump_attr (struct peer *peer, struct attr *attr, char *buf, size_t size) snprintf (buf + strlen (buf), size - strlen (buf), ", atomic-aggregate"); if (CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR))) - snprintf (buf + strlen (buf), size - strlen (buf), ", aggregated by %d %s", + snprintf (buf + strlen (buf), size - strlen (buf), ", aggregated by %u %s", attr->extra->aggregator_as, inet_ntoa (attr->extra->aggregator_addr)); diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c index c08673c..27c3cd6 100644 --- a/bgpd/bgp_ecommunity.c +++ b/bgpd/bgp_ecommunity.c @@ -673,7 +673,7 @@ ecommunity_ecom2str (struct ecommunity *ecom, int format) eas.val = (*pnt++ << 8); eas.val |= (*pnt++); - len = sprintf( str_buf + str_pnt, "%s%d:%d", prefix, + len = sprintf( str_buf + str_pnt, "%s%u:%d", prefix, eas.as, eas.val ); str_pnt += len; first = 0; @@ -688,7 +688,7 @@ ecommunity_ecom2str (struct ecommunity *ecom, int format) eas.val |= (*pnt++ << 8); eas.val |= (*pnt++); - len = sprintf (str_buf + str_pnt, "%s%d:%d", prefix, + len = sprintf (str_buf + str_pnt, "%s%u:%d", prefix, eas.as, eas.val); str_pnt += len; first = 0; diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index ac90f3c..72ad089 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -265,7 +265,7 @@ prefix_rd2str (struct prefix_rd *prd, char *buf, size_t size) if (type == RD_TYPE_AS) { decode_rd_as (pnt + 2, &rd_as); - snprintf (buf, size, "%d:%d", rd_as.as, rd_as.val); + snprintf (buf, size, "%u:%d", rd_as.as, rd_as.val); return buf; } else if (type == RD_TYPE_IP) @@ -371,7 +371,7 @@ show_adj_route_vpn (struct vty *vty, struct peer *peer, structprefix_rd *prd) vty_out (vty, "Route Distinguisher: "); if (type == RD_TYPE_AS) - vty_out (vty, "%d:%d", rd_as.as, rd_as.val); + vty_out (vty, "%u:%d", rd_as.as, rd_as.val); else if (type == RD_TYPE_IP) vty_out (vty, "%s:%d", inet_ntoa (rd_ip.ip), rd_ip.val); @@ -478,7 +478,7 @@ bgp_show_mpls_vpn (struct vty *vty, struct prefix_rd *prd, enumbgp_show_type ty vty_out (vty, "Route Distinguisher: "); if (type == RD_TYPE_AS) - vty_out (vty, "%d:%d", rd_as.as, rd_as.val); + vty_out (vty, "%u:%d", rd_as.as, rd_as.val); else if (type == RD_TYPE_IP) vty_out (vty, "%s:%d", inet_ntoa (rd_ip.ip), rd_ip.val); diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 1422bad..de02bb8 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -813,7 +813,7 @@ bgp_open_send (struct peer *peer) length = bgp_packet_set_size (s); if (BGP_DEBUG (normal, NORMAL)) - zlog_debug ("%s sending OPEN, version %d, my as %d, holdtime %d, id %s", + zlog_debug ("%s sending OPEN, version %d, my as %u, holdtime %d, id %s", peer->host, BGP_VERSION_4, local_as, send_holdtime, inet_ntoa (peer->local_id)); @@ -1184,7 +1184,7 @@ bgp_open_receive (struct peer *peer, bgp_size_t size) /* Receive OPEN message log */ if (BGP_DEBUG (normal, NORMAL)) - zlog_debug ("%s rcv OPEN, version %d, remote-as (in open) %d," + zlog_debug ("%s rcv OPEN, version %d, remote-as (in open) %u," " holdtime %d, id %s", peer->host, version, remote_as, holdtime, inet_ntoa (remote_id)); @@ -1277,7 +1277,7 @@ bgp_open_receive (struct peer *peer, bgp_size_t size) else { if (BGP_DEBUG (normal, NORMAL)) - zlog_debug ("%s bad OPEN, remote AS is %d, expected %d", + zlog_debug ("%s bad OPEN, remote AS is %u, expected %u", peer->host, remote_as, peer->as); bgp_notify_send_with_data (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_BAD_PEER_AS, @@ -1431,7 +1431,7 @@ bgp_open_receive (struct peer *peer, bgp_size_t size) if (remote_as != peer->as) { if (BGP_DEBUG (normal, NORMAL)) - zlog_debug ("%s bad OPEN, remote AS is %d, expected %d", + zlog_debug ("%s bad OPEN, remote AS is %u, expected %u", peer->host, remote_as, peer->as); bgp_notify_send_with_data (peer, BGP_NOTIFY_OPEN_ERR, diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 50407e4..6b7828c 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -834,7 +834,7 @@ bgp_announce_check (struct bgp_info *ri, struct peer *peer,struct prefix *p, { if (BGP_DEBUG (filter, FILTER)) zlog (peer->log, LOG_DEBUG, - "%s [Update:SEND] suppress announcement to peer AS %d is AS path.", + "%s [Update:SEND] suppress announcement to peer AS %u is AS path.", peer->host, peer->as); return 0; } @@ -847,7 +847,7 @@ bgp_announce_check (struct bgp_info *ri, struct peer *peer,struct prefix *p, { if (BGP_DEBUG (filter, FILTER)) zlog (peer->log, LOG_DEBUG, - "%s [Update:SEND] suppress announcement to peer AS %d is AS path.", + "%s [Update:SEND] suppress announcement to peer AS %u is AS path.", peer->host, bgp->confed_id); return 0; @@ -1163,7 +1163,7 @@ bgp_announce_check_rsclient (struct bgp_info *ri, struct peer*rsclient, { if (BGP_DEBUG (filter, FILTER)) zlog (rsclient->log, LOG_DEBUG, - "%s [Update:SEND] suppress announcement to peer AS %d is AS path.", + "%s [Update:SEND] suppress announcement to peer AS %u is AS path.", rsclient->host, rsclient->as); return 0; } @@ -5956,7 +5956,7 @@ route_vty_out_detail (struct vty *vty, struct bgp *bgp, structprefix *p, if (CHECK_FLAG (binfo->flags, BGP_INFO_STALE)) vty_out (vty, ", (stale)"); if (CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR))) - vty_out (vty, ", (aggregated by %d %s)", + vty_out (vty, ", (aggregated by %u %s)", attr->extra->aggregator_as, inet_ntoa (attr->extra->aggregator_addr)); if (CHECK_FLAG (binfo->peer->af_flags[afi][safi], PEER_FLAG_REFLECTOR_CLIENT)) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index bd94c66..96719a1 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -360,11 +360,11 @@ DEFUN (router_bgp, VTY_NEWLINE); return CMD_WARNING; case BGP_ERR_AS_MISMATCH: - vty_out (vty, "BGP is already running; AS is %d%s", as, VTY_NEWLINE); + vty_out (vty, "BGP is already running; AS is %u%s", as, VTY_NEWLINE); return CMD_WARNING; case BGP_ERR_INSTANCE_MISMATCH: vty_out (vty, "BGP view name and AS number mismatch%s", VTY_NEWLINE); - vty_out (vty, "BGP instance is already running; AS is %d%s", + vty_out (vty, "BGP instance is already running; AS is %u%s", as, VTY_NEWLINE); return CMD_WARNING; } @@ -1306,10 +1306,10 @@ peer_remote_as_vty (struct vty *vty, const char *peer_str, switch (ret) { case BGP_ERR_PEER_GROUP_MEMBER: - vty_out (vty, "%% Peer-group AS %d. Cannot configure remote-as for member%s",as, VTY_NEWLINE); + vty_out (vty, "%% Peer-group AS %u. Cannot configureremote-as for member%s", as, VTY_NEWLINE); return CMD_WARNING; case BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT: - vty_out (vty, "%% The AS# can not be changed from %d to %s, peer-group membersmust be all internal or all external%s", as, as_str, VTY_NEWLINE); + vty_out(vty, "%% The AS# can not be changed from %u to %s, peer-group members must be allinternal or all external%s", as, as_str, VTY_NEWLINE); return CMD_WARNING; } return bgp_vty_return (vty, ret); @@ -1647,7 +1647,7 @@ DEFUN (neighbor_set_peer_group, if (ret == BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT) { - vty_out (vty, "%% Peer with AS %d cannot be in this peer-group, members mustbe all internal or all external%s", as, VTY_NEWLINE); + vty_out (vty, "%% Peerwith AS %u cannot be in this peer-group, members must be all internal or allexternal%s", as, VTY_NEWLINE); return CMD_WARNING; } @@ -6912,7 +6912,7 @@ bgp_show_summary (struct vty *vty, struct bgp *bgp, int afi,int safi) /* Usage summary and header */ vty_out (vty, - "BGP router identifier %s, local AS number %d%s", + "BGP router identifier %s, local AS number %u%s", inet_ntoa (bgp->router_id), bgp->as, VTY_NEWLINE); ents = bgp_table_count (bgp->rib[afi][safi]); @@ -6959,7 +6959,7 @@ bgp_show_summary (struct vty *vty, struct bgp *bgp, int afi,int safi) vty_out (vty, "4 "); - vty_out (vty, "%5d %7d %7d %8d %4d %4lu ", + vty_out (vty, "%5u %7d %7d %8d %4d %4lu ", peer->as, peer->open_in + peer->update_in + peer->keepalive_in + peer->notify_in + peer->refresh_in + peer->dynamic_cap_in, @@ -7469,8 +7469,8 @@ bgp_show_peer (struct vty *vty, struct peer *p) /* Configured IP address. */ vty_out (vty, "BGP neighbor is %s, ", p->host); - vty_out (vty, "remote AS %d, ", p->as); - vty_out (vty, "local AS %d%s, ", + vty_out (vty, "remote AS %u, ", p->as); + vty_out (vty, "local AS %u%s, ", p->change_local_as ? p->change_local_as : p->local_as, CHECK_FLAG (p->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND) ? " no-prepend" : ""); @@ -8252,7 +8252,7 @@ bgp_show_rsclient_summary (struct vty *vty, struct bgp *bgp, "Route Server's BGP router identifier %s%s", inet_ntoa (bgp->router_id), VTY_NEWLINE); vty_out (vty, - "Route Server's local AS number %d%s", bgp->as, + "Route Server's local AS number %u%s", bgp->as, VTY_NEWLINE); vty_out (vty, "%s", VTY_NEWLINE); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 8eb0d2e..cebde0a 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -4512,13 +4512,13 @@ bgp_config_write_peer (struct vty *vty, struct bgp *bgp, vty_out (vty, " neighbor %s peer-group%s", addr, VTY_NEWLINE); if (peer->as) - vty_out (vty, " neighbor %s remote-as %d%s", addr, peer->as, + vty_out (vty, " neighbor %s remote-as %u%s", addr, peer->as, VTY_NEWLINE); } else { if (! g_peer->as) - vty_out (vty, " neighbor %s remote-as %d%s", addr, peer->as, + vty_out (vty, " neighbor %s remote-as %u%s", addr, peer->as, VTY_NEWLINE); if (peer->af_group[AFI_IP][SAFI_UNICAST]) vty_out (vty, " neighbor %s peer-group %s%s", addr, @@ -4528,7 +4528,7 @@ bgp_config_write_peer (struct vty *vty, struct bgp *bgp, /* local-as. */ if (peer->change_local_as) if (! peer_group_active (peer)) - vty_out (vty, " neighbor %s local-as %d%s%s", addr, + vty_out (vty, " neighbor %s local-as %u%s%s", addr, peer->change_local_as, CHECK_FLAG (peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND) ? " no-prepend" : "", VTY_NEWLINE); @@ -4917,7 +4917,7 @@ bgp_config_write (struct vty *vty) vty_out (vty, "!%s", VTY_NEWLINE); /* Router bgp ASN */ - vty_out (vty, "router bgp %d", bgp->as); + vty_out (vty, "router bgp %u", bgp->as); if (bgp_option_check (BGP_OPT_MULTIPLE_INSTANCE)) { @@ -4978,7 +4978,7 @@ bgp_config_write (struct vty *vty) vty_out (vty, " bgp confederation peers"); for (i = 0; i < bgp->confed_peers_cnt; i++) - vty_out(vty, " %d", bgp->confed_peers[i]); + vty_out(vty, " %u", bgp->confed_peers[i]); vty_out (vty, "%s", VTY_NEWLINE); } -- 1.6.0.6 _______________________________________________ Quagga-dev mailing list Quagga-dev@lists.quagga.net http://lists.quagga.net/mailman/listinfo/quagga-dev <b>[prev in list] [next in list] [<font color="#c0c0c0">prev in thread</font>] [<font color="#c0c0c0">next in thread</font>] </b> </pre> </pre><br><center> Configure | About | News | Donate | Addalist | Sponsors:10East,KoreLogic,Terra-International,Chakpak.com </center> </body> </html>

References:

http://www.debian.org/security/2009/dsa-1788
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=526311
http://xforce.iss.net/xforce/xfdb/50317
http://www.securityfocus.com/bid/34817
http://www.osvdb.org/54200
http://www.openwall.com/lists/oss-security/2009/05/01/2
http://www.openwall.com/lists/oss-security/2009/05/01/1
http://thread.gmane.org/gmane.network.quagga.devel/6513
http://secunia.com/advisories/34999
http://marc.info/?l=quagga-dev&m=123364779626078&w=2


Vote for this issue:
50%
50%


 

Thanks for you vote!


 

Thanks for you comment!
Your message is in quarantine 48 hours.

Comment it here.


(*) - required fields.  
{{ x.nick }} | Date: {{ x.ux * 1000 | date:'yyyy-MM-dd' }} {{ x.ux * 1000 | date:'HH:mm' }} CET+1
{{ x.comment }}

Copyright 2019, cxsecurity.com

 

Back to Top