Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: A couple more bgpd crashes on malformed attributes (backport #14645) #14654

Merged
merged 2 commits into from
Oct 25, 2023
Merged

bgpd: A couple more bgpd crashes on malformed attributes (backport #14645) #14654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d19727b
bgpd: Handle MP_REACH_NLRI malformed packets with session reset
ton31337 Oct 20, 2023
77e4046
bgpd: Check mandatory attributes more carefully for UPDATE message
ton31337 Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions bgpd/bgp_attr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2388,7 +2388,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,

mp_update->afi = afi;
mp_update->safi = safi;
return BGP_ATTR_PARSE_EOR;
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_MAL_ATTR, 0);
}

mp_update->afi = afi;
Expand Down Expand Up @@ -3349,13 +3349,15 @@ bgp_attr_unknown(struct bgp_attr_parser_args *args)
}

/* Well-known attribute check. */
static int bgp_attr_check(struct peer *peer, struct attr *attr)
static int bgp_attr_check(struct peer *peer, struct attr *attr,
bgp_size_t length)
{
uint8_t type = 0;

/* BGP Graceful-Restart End-of-RIB for IPv4 unicast is signaled as an
* empty UPDATE. */
if (CHECK_FLAG(peer->cap, PEER_CAP_RESTART_RCV) && !attr->flag)
if (CHECK_FLAG(peer->cap, PEER_CAP_RESTART_RCV) && !attr->flag &&
!length)
return BGP_ATTR_PARSE_PROCEED;

/* "An UPDATE message that contains the MP_UNREACH_NLRI is not required
Expand Down Expand Up @@ -3407,7 +3409,7 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
enum bgp_attr_parse_ret ret;
uint8_t flag = 0;
uint8_t type = 0;
bgp_size_t length;
bgp_size_t length = 0;
uint8_t *startp, *endp;
uint8_t *attr_endp;
uint8_t seen[BGP_ATTR_BITMAP_SIZE];
Expand Down Expand Up @@ -3658,10 +3660,6 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
goto done;
}

if (ret == BGP_ATTR_PARSE_EOR) {
goto done;
}

if (ret == BGP_ATTR_PARSE_ERROR) {
flog_warn(EC_BGP_ATTRIBUTE_PARSE_ERROR,
"%s: Attribute %s, parse error", peer->host,
Expand Down Expand Up @@ -3734,7 +3732,7 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
}

/* Check all mandatory well-known attributes are present */
ret = bgp_attr_check(peer, attr);
ret = bgp_attr_check(peer, attr, length);
if (ret < 0)
goto done;

Expand Down
1 change: 0 additions & 1 deletion bgpd/bgp_attr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,6 @@ enum bgp_attr_parse_ret {
/* only used internally, send notify + convert to BGP_ATTR_PARSE_ERROR
*/
BGP_ATTR_PARSE_ERROR_NOTIFYPLS = -3,
BGP_ATTR_PARSE_EOR = -4,
};

struct bpacket_attr_vec_arr;
Expand Down
6 changes: 1 addition & 5 deletions bgpd/bgp_packet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2053,8 +2053,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
* Non-MP IPv4/Unicast EoR is a completely empty UPDATE
* and MP EoR should have only an empty MP_UNREACH
*/
if ((!update_len && !withdraw_len && nlris[NLRI_MP_UPDATE].length == 0)
|| (attr_parse_ret == BGP_ATTR_PARSE_EOR)) {
if (!update_len && !withdraw_len && nlris[NLRI_MP_UPDATE].length == 0) {
afi_t afi = 0;
safi_t safi;
struct graceful_restart_info *gr_info;
Expand All @@ -2075,9 +2074,6 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
&& nlris[NLRI_MP_WITHDRAW].length == 0) {
afi = nlris[NLRI_MP_WITHDRAW].afi;
safi = nlris[NLRI_MP_WITHDRAW].safi;
} else if (attr_parse_ret == BGP_ATTR_PARSE_EOR) {
afi = nlris[NLRI_MP_UPDATE].afi;
safi = nlris[NLRI_MP_UPDATE].safi;
}

if (afi && peer->afc[afi][safi]) {
Expand Down
Loading