On Tue, Mar 21, 2023 at 10:36:16AM +0800, cjt wrote:
> Hello, I have find a bug in openbgpd. RFC 4760 says that the length of
> MP_REACH_NLRI is bigger than 5.
> +---------------------------------------------------------+
> | Address Family Identifier (2 octets) |
> +---------------------------------------------------------+
> | Subsequent Address Family Identifier (1 octet) |
> +---------------------------------------------------------+
> | Length of Next Hop Network Address (1 octet) |
> +---------------------------------------------------------+
> | Network Address of Next Hop (variable) |
> +---------------------------------------------------------+
> | Reserved (1 octet) |
> +---------------------------------------------------------+
> | Network Layer Reachability Information (variable) |
> +---------------------------------------------------------+
> While, I found that in the code snippet of Function "rde_attr_parse" only
> check 4 bytes.
>
>
> rde_attr_parse(){
> case ATTR_MP_REACH_NLRI:
> if (attr_len < 4)
> goto bad_len;
> break;
> }
> When I construct a MP_REACH_NLRI packet only have four bytes, it will make
> the rde_get_mp_nexthop function without check the length of the data,
> directly +1 to the "totlen". And make the "pos" is bigger than the "mplen"
> in the rde_update_dispatch. As "mplen" is unsigned int , it will be a large
> number and execute the following code.
> int rde_get_mp_nexthop(u_char *data, uint16_t len, uint8_t aid, struct
> filterstate *state){
>
> /* ignore reserved (old SNPA) field as per RFC4760 */
> totlen += nhlen + 1;
> data += nhlen + 1;
> return (totlen);
> }
>
>
> void rde_update_dispatch(struct rde_peer *peer, struct imsg *imsg){
> if ((pos = rde_get_mp_nexthop(mpp, mplen, aid, &state)) == -1) {
> ....
> goto done;
> }
> mpp += pos;
> mplen -= pos;
> while (mplen > 0) {
> switch (aid) {
> case AID_INET6:
> ....
> }
> }
Thanks for the report. The following diff should fix the issue.
--
:wq Claudio
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.596
diff -u -p -r1.596 rde.c
--- rde.c 13 Mar 2023 16:52:42 -0000 1.596
+++ rde.c 21 Mar 2023 08:00:45 -0000
@@ -2117,7 +2117,7 @@ bad_flags:
goto bad_flags;
goto optattr;
case ATTR_MP_REACH_NLRI:
- if (attr_len < 4)
+ if (attr_len < 5)
goto bad_len;
if (!CHECK_FLAGS(flags, ATTR_OPTIONAL, 0))
goto bad_flags;
@@ -2310,7 +2310,7 @@ rde_get_mp_nexthop(u_char *data, uint16_
totlen = 1;
len--;
- if (nhlen > len)
+ if (nhlen + 1 > len)
return (-1);
memset(&nexthop, 0, sizeof(nexthop));
