diff options
| -rw-r--r-- | bgpd/ChangeLog | 20 | ||||
| -rw-r--r-- | bgpd/bgp_attr.c | 24 | ||||
| -rw-r--r-- | bgpd/bgp_attr.h | 2 | ||||
| -rw-r--r-- | bgpd/bgp_open.c | 2 | ||||
| -rw-r--r-- | bgpd/bgp_packet.c | 13 | ||||
| -rw-r--r-- | tests/ChangeLog | 4 | ||||
| -rw-r--r-- | tests/bgp_capability_test.c | 30 |
7 files changed, 87 insertions, 8 deletions
diff --git a/bgpd/ChangeLog b/bgpd/ChangeLog index 3fa3837a..70bcc0fb 100644 --- a/bgpd/ChangeLog +++ b/bgpd/ChangeLog @@ -1,3 +1,23 @@ +2007-12-22 Paul Jakma <paul.jakma@sun.com> + + * Fix series of vulnerabilities reported by "Mu Security + Research Team", where bgpd can be made to crash by sending + malformed packets - requires that bgpd be configured with a + session to the peer. + * bgp_attr.c: (bgp_attr_as4_path) aspath_parse may fail, only + set the attribute flag indicating AS4_PATH if we actually managed + to parse one. + (bgp_attr_munge_as4_attrs) Assert was too general, it is possible + to receive AS4_AGGREGATOR before AGGREGATOR. + (bgp_attr_parse) Check that we have actually received the extra + byte of header for Extended-Length attributes. + * bgp_attr.h: Fix BGP_ATTR_MIN_LEN to account for the length byte. + * bgp_open.c: (cap_minsizes) Fix size of CAPABILITY_CODE_RESTART, + incorrect -2 left in place from a development version of as4-path + patch. + * bgp_packet.c: (bgp_route_refresh_receive) ORF length parameter + needs to be properly sanity checked. + 2007-12-18 Denis Ovsienko * bgp_routemap.c: (no_set_aspath_prepend) This command cancelled diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index b463b3c0..dd3cc965 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -892,7 +892,8 @@ bgp_attr_as4_path (struct peer *peer, bgp_size_t length, *as4_path = aspath_parse (peer->ibuf, length, 1); /* Set aspath attribute flag. */ - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH); + if (as4_path) + attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH); return 0; } @@ -1126,10 +1127,10 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, */ if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR) ) ) { - assert (attre); - if ( attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) ) { + assert (attre); + /* received both. * if the as_number in aggregator is not AS_TRANS, * then AS4_AGGREGATOR and AS4_PATH shall be ignored @@ -1170,7 +1171,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, zlog_debug ("[AS4] %s BGP not AS4 capable peer send" " AS4_AGGREGATOR but no AGGREGATOR, will take" " it as if AGGREGATOR with AS_TRANS had been there", peer->host); - attre->aggregator_as = as4_aggregator; + (attre = bgp_attr_extra_get (attr))->aggregator_as = as4_aggregator; /* sweep it under the carpet and simulate a "good" AGGREGATOR */ attr->flag |= (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR)); } @@ -1543,6 +1544,21 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, flag = stream_getc (BGP_INPUT (peer)); type = stream_getc (BGP_INPUT (peer)); + /* Check whether Extended-Length applies and is in bounds */ + if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) + && ((endp - startp) < (BGP_ATTR_MIN_LEN + 1))) + { + zlog (peer->log, LOG_WARNING, + "%s Extended length set, but just %u bytes of attr header", + peer->host, + (unsigned long) (endp - STREAM_PNT (BGP_INPUT (peer)))); + + bgp_notify_send (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); + return -1; + } + /* Check extended attribue length bit. */ if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN)) length = stream_getw (BGP_INPUT (peer)); diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 1af9ce30..e152b9f4 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -44,7 +44,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA #define BGP_ATTR_FLAG_EXTLEN 0x10 /* Extended length flag. */ /* BGP attribute header must bigger than 2. */ -#define BGP_ATTR_MIN_LEN 2 /* Attribute flag and type. */ +#define BGP_ATTR_MIN_LEN 3 /* Attribute flag, type length. */ #define BGP_ATTR_DEFAULT_WEIGHT 32768 /* Additional/uncommon BGP attributes. diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index 38431d4c..1b13a458 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -461,7 +461,7 @@ static size_t cap_minsizes[] = [CAPABILITY_CODE_MP] = sizeof (struct capability_mp_data), [CAPABILITY_CODE_REFRESH] = CAPABILITY_CODE_REFRESH_LEN, [CAPABILITY_CODE_ORF] = sizeof (struct capability_orf_entry), - [CAPABILITY_CODE_RESTART] = sizeof (struct capability_gr) - 2, + [CAPABILITY_CODE_RESTART] = sizeof (struct capability_gr), [CAPABILITY_CODE_AS4] = CAPABILITY_CODE_AS4_LEN, [CAPABILITY_CODE_DYNAMIC] = CAPABILITY_CODE_DYNAMIC_LEN, [CAPABILITY_CODE_REFRESH_OLD] = CAPABILITY_CODE_REFRESH_LEN, diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 1fa2fdfd..8319a885 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -1960,11 +1960,14 @@ bgp_route_refresh_receive (struct peer *peer, bgp_size_t size) when_to_refresh = stream_getc (s); end = stream_pnt (s) + (size - 5); - while (stream_pnt (s) < end) + while ((stream_pnt (s) + 2) < end) { orf_type = stream_getc (s); orf_len = stream_getw (s); - + + /* orf_len in bounds? */ + if ((stream_pnt (s) + orf_len) > end) + break; /* XXX: Notify instead?? */ if (orf_type == ORF_TYPE_PREFIX || orf_type == ORF_TYPE_PREFIX_OLD) { @@ -1984,6 +1987,12 @@ bgp_route_refresh_receive (struct peer *peer, bgp_size_t size) peer->host, orf_type, orf_len); } + /* we're going to read at least 1 byte of common ORF header, + * and 7 bytes of ORF Address-filter entry from the stream + */ + if (orf_len < 7) + break; + /* ORF prefix-list name */ sprintf (name, "%s.%d.%d", peer->host, afi, safi); diff --git a/tests/ChangeLog b/tests/ChangeLog index 94f58749..16412bde 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,7 @@ +2007-12-22 Paul Jakma <paul.jakma@sun.com> + + * bgp_capability_test.c: Test for empty capabilities. + 2007-09-27 Paul Jakma <paul.jakma@sun.com> * aspath_test.c: Test dupe-weeding from sets. diff --git a/tests/bgp_capability_test.c b/tests/bgp_capability_test.c index 6771b579..0dbf4fb9 100644 --- a/tests/bgp_capability_test.c +++ b/tests/bgp_capability_test.c @@ -362,6 +362,36 @@ static struct test_segment misc_segments[] = }, 15, SHOULD_ERR, }, + { "GR-empty", + "GR capability, but empty.", + { /* hdr */ 0x40, 0x0, + }, + 2, SHOULD_ERR, + }, + { "MP-empty", + "MP capability, but empty.", + { /* hdr */ 0x1, 0x0, + }, + 2, SHOULD_ERR, + }, + { "ORF-empty", + "ORF capability, but empty.", + { /* hdr */ 0x3, 0x0, + }, + 2, SHOULD_ERR, + }, + { "AS4-empty", + "AS4 capability, but empty.", + { /* hdr */ 0x41, 0x0, + }, + 2, SHOULD_ERR, + }, + { "dyn-empty", + "Dynamic capability, but empty.", + { /* hdr */ 0x42, 0x0, + }, + 2, SHOULD_PARSE, + }, { "dyn-old", "Dynamic capability (deprecated version)", { CAPABILITY_CODE_DYNAMIC, 0x0 }, |