|
| 1 | +From 3e46b43e3788f0f87bae56a86b54d412b4710286 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Donald Sharp < [email protected]> |
| 3 | +Date: Fri, 30 Sep 2022 08:51:45 -0400 |
| 4 | +Subject: [PATCH 1/2] bgpd: Ensure FRR has enough data to read 2 bytes in |
| 5 | + peek_for_as4_capability |
| 6 | + |
| 7 | +In peek_for_as4_capability the code is checking that the |
| 8 | +stream has at least 2 bytes to read ( the opt_type and the |
| 9 | +opt_length ). However if BGP_OPEN_EXT_OPT_PARAMS_CAPABLE(peer) |
| 10 | +is configured then FRR is reading 3 bytes. Which is not good |
| 11 | +since the packet could be badly formated. Ensure that |
| 12 | +FRR has the appropriate data length to read the data. |
| 13 | + |
| 14 | +Signed-off-by: Donald Sharp < [email protected]> |
| 15 | +--- |
| 16 | + bgpd/bgp_open.c | 27 +++++++++++++++++++++------ |
| 17 | + 1 file changed, 21 insertions(+), 6 deletions(-) |
| 18 | + |
| 19 | +diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c |
| 20 | +index 7248f034a5a..a760a7ca013 100644 |
| 21 | +--- a/bgpd/bgp_open.c |
| 22 | ++++ b/bgpd/bgp_open.c |
| 23 | +@@ -1185,15 +1185,30 @@ as_t peek_for_as4_capability(struct peer *peer, uint16_t length) |
| 24 | + uint8_t opt_type; |
| 25 | + uint16_t opt_length; |
| 26 | + |
| 27 | +- /* Check the length. */ |
| 28 | +- if (stream_get_getp(s) + 2 > end) |
| 29 | ++ /* Ensure we can read the option type */ |
| 30 | ++ if (stream_get_getp(s) + 1 > end) |
| 31 | + goto end; |
| 32 | + |
| 33 | +- /* Fetch option type and length. */ |
| 34 | ++ /* Fetch the option type */ |
| 35 | + opt_type = stream_getc(s); |
| 36 | +- opt_length = BGP_OPEN_EXT_OPT_PARAMS_CAPABLE(peer) |
| 37 | +- ? stream_getw(s) |
| 38 | +- : stream_getc(s); |
| 39 | ++ |
| 40 | ++ /* |
| 41 | ++ * Check the length and fetch the opt_length |
| 42 | ++ * If the peer is BGP_OPEN_EXT_OPT_PARAMS_CAPABLE(peer) |
| 43 | ++ * then we do a getw which is 2 bytes. So we need to |
| 44 | ++ * ensure that we can read that as well |
| 45 | ++ */ |
| 46 | ++ if (BGP_OPEN_EXT_OPT_PARAMS_CAPABLE(peer)) { |
| 47 | ++ if (stream_get_getp(s) + 2 > end) |
| 48 | ++ goto end; |
| 49 | ++ |
| 50 | ++ opt_length = stream_getw(s); |
| 51 | ++ } else { |
| 52 | ++ if (stream_get_getp(s) + 1 > end) |
| 53 | ++ goto end; |
| 54 | ++ |
| 55 | ++ opt_length = stream_getc(s); |
| 56 | ++ } |
| 57 | + |
| 58 | + /* Option length check. */ |
| 59 | + if (stream_get_getp(s) + opt_length > end) |
| 60 | + |
| 61 | +From 1117baca3c592877a4d8a13ed6a1d9bd83977487 Mon Sep 17 00:00:00 2001 |
| 62 | +From: Donald Sharp < [email protected]> |
| 63 | +Date: Fri, 30 Sep 2022 08:57:43 -0400 |
| 64 | +Subject: [PATCH 2/2] bgpd: Ensure FRR has enough data to read 2 bytes in |
| 65 | + bgp_open_option_parse |
| 66 | + |
| 67 | +In bgp_open_option_parse the code is checking that the |
| 68 | +stream has at least 2 bytes to read ( the opt_type and |
| 69 | +the opt_length). However if BGP_OPEN_EXT_OPT_PARAMS_CAPABLE(peer) |
| 70 | +is configured then FRR is reading 3 bytes. Which is not good |
| 71 | +since the packet could be badly formateed. Ensure that |
| 72 | +FRR has the appropriate data length to read the data. |
| 73 | + |
| 74 | +Signed-off-by: Donald Sharp < [email protected]> |
| 75 | +--- |
| 76 | + bgpd/bgp_open.c | 35 ++++++++++++++++++++++++++++------- |
| 77 | + 1 file changed, 28 insertions(+), 7 deletions(-) |
| 78 | + |
| 79 | +diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c |
| 80 | +index a760a7ca013..d1667fac261 100644 |
| 81 | +--- a/bgpd/bgp_open.c |
| 82 | ++++ b/bgpd/bgp_open.c |
| 83 | +@@ -1278,19 +1278,40 @@ int bgp_open_option_parse(struct peer *peer, uint16_t length, |
| 84 | + uint8_t opt_type; |
| 85 | + uint16_t opt_length; |
| 86 | + |
| 87 | +- /* Must have at least an OPEN option header */ |
| 88 | +- if (STREAM_READABLE(s) < 2) { |
| 89 | ++ /* |
| 90 | ++ * Check that we can read the opt_type and fetch it |
| 91 | ++ */ |
| 92 | ++ if (STREAM_READABLE(s) < 1) { |
| 93 | + zlog_info("%s Option length error", peer->host); |
| 94 | + bgp_notify_send(peer, BGP_NOTIFY_OPEN_ERR, |
| 95 | + BGP_NOTIFY_OPEN_MALFORMED_ATTR); |
| 96 | + return -1; |
| 97 | + } |
| 98 | +- |
| 99 | +- /* Fetch option type and length. */ |
| 100 | + opt_type = stream_getc(s); |
| 101 | +- opt_length = BGP_OPEN_EXT_OPT_PARAMS_CAPABLE(peer) |
| 102 | +- ? stream_getw(s) |
| 103 | +- : stream_getc(s); |
| 104 | ++ |
| 105 | ++ /* |
| 106 | ++ * Check the length of the stream to ensure that |
| 107 | ++ * FRR can properly read the opt_length. Then read it |
| 108 | ++ */ |
| 109 | ++ if (BGP_OPEN_EXT_OPT_PARAMS_CAPABLE(peer)) { |
| 110 | ++ if (STREAM_READABLE(s) < 2) { |
| 111 | ++ zlog_info("%s Option length error", peer->host); |
| 112 | ++ bgp_notify_send(peer, BGP_NOTIFY_OPEN_ERR, |
| 113 | ++ BGP_NOTIFY_OPEN_MALFORMED_ATTR); |
| 114 | ++ return -1; |
| 115 | ++ } |
| 116 | ++ |
| 117 | ++ opt_length = stream_getw(s); |
| 118 | ++ } else { |
| 119 | ++ if (STREAM_READABLE(s) < 1) { |
| 120 | ++ zlog_info("%s Option length error", peer->host); |
| 121 | ++ bgp_notify_send(peer, BGP_NOTIFY_OPEN_ERR, |
| 122 | ++ BGP_NOTIFY_OPEN_MALFORMED_ATTR); |
| 123 | ++ return -1; |
| 124 | ++ } |
| 125 | ++ |
| 126 | ++ opt_length = stream_getc(s); |
| 127 | ++ } |
| 128 | + |
| 129 | + /* Option length check. */ |
| 130 | + if (STREAM_READABLE(s) < opt_length) { |
0 commit comments