mirror of
https://git.proxmox.com/git/mirror_frr
synced 2025-11-02 21:58:04 +00:00
Merge pull request #14175 from samanvithab/bgpd_update_err_fix
bgpd: Few fixes for Update message error handling of malformed attribute
This commit is contained in:
Russ White
GitHub
commit
2bc2ff61c8
158
bgpd/bgp_attr.c
158
bgpd/bgp_attr.c
@ -3507,27 +3507,6 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
|
||||
else
|
||||
length = stream_getc(BGP_INPUT(peer));
|
||||
|
||||
/* If any attribute appears more than once in the UPDATE
|
||||
message, then the Error Subcode is set to Malformed Attribute
|
||||
List. */
|
||||
|
||||
if (CHECK_BITMAP(seen, type)) {
|
||||
flog_warn(
|
||||
EC_BGP_ATTRIBUTE_REPEATED,
|
||||
"%s: error BGP attribute type %d appears twice in a message",
|
||||
peer->host, type);
|
||||
|
||||
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
|
||||
BGP_NOTIFY_UPDATE_MAL_ATTR);
|
||||
ret = BGP_ATTR_PARSE_ERROR;
|
||||
goto done;
|
||||
}
|
||||
|
||||
/* Set type to bitmap to check duplicate attribute. `type' is
|
||||
unsigned char so it never overflow bitmap range. */
|
||||
|
||||
SET_BITMAP(seen, type);
|
||||
|
||||
/* Overflow check. */
|
||||
attr_endp = BGP_INPUT_PNT(peer) + length;
|
||||
|
||||
@ -3537,50 +3516,107 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
|
||||
"%s: BGP type %d length %d is too large, attribute total length is %d. attr_endp is %p. endp is %p",
|
||||
peer->host, type, length, size, attr_endp,
|
||||
endp);
|
||||
/*
|
||||
* RFC 4271 6.3
|
||||
* If any recognized attribute has an Attribute
|
||||
* Length that conflicts with the expected length
|
||||
* (based on the attribute type code), then the
|
||||
* Error Subcode MUST be set to Attribute Length
|
||||
* Error. The Data field MUST contain the erroneous
|
||||
* attribute (type, length, and value).
|
||||
* ----------
|
||||
* We do not currently have a good way to determine the
|
||||
* length of the attribute independent of the length
|
||||
* received in the message. Instead we send the
|
||||
* minimum between the amount of data we have and the
|
||||
* amount specified by the attribute length field.
|
||||
*
|
||||
* Instead of directly passing in the packet buffer and
|
||||
* offset we use the stream_get* functions to read into
|
||||
* a stack buffer, since they perform bounds checking
|
||||
* and we are working with untrusted data.
|
||||
*/
|
||||
unsigned char ndata[peer->max_packet_size];
|
||||
memset(ndata, 0x00, sizeof(ndata));
|
||||
size_t lfl =
|
||||
CHECK_FLAG(flag, BGP_ATTR_FLAG_EXTLEN) ? 2 : 1;
|
||||
/* Rewind to end of flag field */
|
||||
stream_rewind_getp(BGP_INPUT(peer), (1 + lfl));
|
||||
/* Type */
|
||||
stream_get(&ndata[0], BGP_INPUT(peer), 1);
|
||||
/* Length */
|
||||
stream_get(&ndata[1], BGP_INPUT(peer), lfl);
|
||||
/* Value */
|
||||
size_t atl = attr_endp - startp;
|
||||
size_t ndl = MIN(atl, STREAM_READABLE(BGP_INPUT(peer)));
|
||||
stream_get(&ndata[lfl + 1], BGP_INPUT(peer), ndl);
|
||||
|
||||
bgp_notify_send_with_data(
|
||||
peer, BGP_NOTIFY_UPDATE_ERR,
|
||||
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, ndata,
|
||||
ndl + lfl + 1);
|
||||
/* Only relax error handling for eBGP peers */
|
||||
if (peer->sort != BGP_PEER_EBGP) {
|
||||
/*
|
||||
* RFC 4271 6.3
|
||||
* If any recognized attribute has an Attribute
|
||||
* Length that conflicts with the expected length
|
||||
* (based on the attribute type code), then the
|
||||
* Error Subcode MUST be set to Attribute Length
|
||||
* Error. The Data field MUST contain the erroneous
|
||||
* attribute (type, length, and value).
|
||||
* ----------
|
||||
* We do not currently have a good way to determine the
|
||||
* length of the attribute independent of the length
|
||||
* received in the message. Instead we send the
|
||||
* minimum between the amount of data we have and the
|
||||
* amount specified by the attribute length field.
|
||||
*
|
||||
* Instead of directly passing in the packet buffer and
|
||||
* offset we use the stream_get* functions to read into
|
||||
* a stack buffer, since they perform bounds checking
|
||||
* and we are working with untrusted data.
|
||||
*/
|
||||
unsigned char ndata[peer->max_packet_size];
|
||||
|
||||
ret = BGP_ATTR_PARSE_ERROR;
|
||||
goto done;
|
||||
memset(ndata, 0x00, sizeof(ndata));
|
||||
size_t lfl =
|
||||
CHECK_FLAG(flag, BGP_ATTR_FLAG_EXTLEN) ? 2 : 1;
|
||||
/* Rewind to end of flag field */
|
||||
stream_rewind_getp(BGP_INPUT(peer), (1 + lfl));
|
||||
/* Type */
|
||||
stream_get(&ndata[0], BGP_INPUT(peer), 1);
|
||||
/* Length */
|
||||
stream_get(&ndata[1], BGP_INPUT(peer), lfl);
|
||||
/* Value */
|
||||
size_t atl = attr_endp - startp;
|
||||
size_t ndl = MIN(atl, STREAM_READABLE(BGP_INPUT(peer)));
|
||||
|
||||
stream_get(&ndata[lfl + 1], BGP_INPUT(peer), ndl);
|
||||
|
||||
bgp_notify_send_with_data(
|
||||
peer, BGP_NOTIFY_UPDATE_ERR,
|
||||
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, ndata,
|
||||
ndl + lfl + 1);
|
||||
|
||||
ret = BGP_ATTR_PARSE_ERROR;
|
||||
goto done;
|
||||
} else {
|
||||
/* Handling as per RFC7606 section 4, treat-as-withdraw approach
|
||||
* must be followed when the total attribute length is in conflict
|
||||
* with the enclosed path attribute length.
|
||||
*/
|
||||
flog_warn(
|
||||
EC_BGP_ATTRIBUTE_PARSE_WITHDRAW,
|
||||
"%s: Attribute %s, parse error - treating as withdrawal",
|
||||
peer->host, lookup_msg(attr_str, type, NULL));
|
||||
ret = BGP_ATTR_PARSE_WITHDRAW;
|
||||
stream_forward_getp(BGP_INPUT(peer), endp - BGP_INPUT_PNT(peer));
|
||||
goto done;
|
||||
}
|
||||
}
|
||||
|
||||
/* If attribute appears more than once in the UPDATE message,
|
||||
* for MP_REACH_NLRI & MP_UNREACH_NLRI attributes
|
||||
* the Error Subcode is set to Malformed Attribute List.
|
||||
* For all other attributes, all the occurances of the attribute
|
||||
* other than the first occurence is discarded. (RFC7606 3g)
|
||||
*/
|
||||
|
||||
if (CHECK_BITMAP(seen, type)) {
|
||||
/* Only relax error handling for eBGP peers */
|
||||
if (peer->sort != BGP_PEER_EBGP ||
|
||||
type == BGP_ATTR_MP_REACH_NLRI || type == BGP_ATTR_MP_UNREACH_NLRI) {
|
||||
flog_warn(
|
||||
EC_BGP_ATTRIBUTE_REPEATED,
|
||||
"%s: error BGP attribute type %d appears twice in a message",
|
||||
peer->host, type);
|
||||
|
||||
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
|
||||
BGP_NOTIFY_UPDATE_MAL_ATTR);
|
||||
ret = BGP_ATTR_PARSE_ERROR;
|
||||
goto done;
|
||||
} else {
|
||||
flog_warn(
|
||||
EC_BGP_ATTRIBUTE_REPEATED,
|
||||
"%s: error BGP attribute type %d appears twice in a message - discard attribute",
|
||||
peer->host, type);
|
||||
/* Adjust the stream getp to the end of the attribute, in case we
|
||||
* haven't read all the attributes.
|
||||
*/
|
||||
stream_set_getp(BGP_INPUT(peer),
|
||||
(startp - STREAM_DATA(BGP_INPUT(peer))) + (attr_endp - startp));
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
/* Set type to bitmap to check duplicate attribute. `type' is
|
||||
unsigned char so it never overflow bitmap range. */
|
||||
|
||||
SET_BITMAP(seen, type);
|
||||
|
||||
struct bgp_attr_parser_args attr_args = {
|
||||
.peer = peer,
|
||||
.length = length,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user