From d91788284ed910bcf945c01ceb18334423cc352d Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Tue, 9 Feb 2016 15:23:03 +0000 Subject: [PATCH] lib: Check prefix length from zebra is sensible * zclient.c: prefix length on router-id and interface address add messages not sanity checked. fix. * */*_zebra.c: Prefix length on zebra route read was not checked, and clients use it to write to storage. An evil zebra could overflow client structures by sending overly long prefixlen. Prompted by discussions with: Donald Sharp --- bgpd/bgp_zebra.c | 4 ++-- isisd/isis_zebra.c | 2 +- lib/zclient.c | 45 +++++++++++++++++++++++++++----------------- ospf6d/ospf6_zebra.c | 2 +- ospfd/ospf_zebra.c | 2 +- ripd/rip_zebra.c | 2 +- ripngd/ripng_zebra.c | 2 +- 7 files changed, 35 insertions(+), 24 deletions(-) diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 15db32155..0f5c3ce7a 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -607,7 +607,7 @@ zebra_read_ipv4 (int command, struct zclient *zclient, zebra_size_t length, /* IPv4 prefix. */ memset (&p, 0, sizeof (struct prefix_ipv4)); p.family = AF_INET; - p.prefixlen = stream_getc (s); + p.prefixlen = MIN(IPV4_MAX_PREFIXLEN, stream_getc (s)); stream_get (&p.prefix, s, PSIZE (p.prefixlen)); /* Nexthop, ifindex, distance, metric. */ @@ -722,7 +722,7 @@ zebra_read_ipv6 (int command, struct zclient *zclient, zebra_size_t length, /* IPv6 prefix. */ memset (&p, 0, sizeof (struct prefix_ipv6)); p.family = AF_INET6; - p.prefixlen = stream_getc (s); + p.prefixlen = MIN(IPV6_MAX_PREFIXLEN, stream_getc (s)); stream_get (&p.prefix, s, PSIZE (p.prefixlen)); /* Nexthop, ifindex, distance, metric. */ diff --git a/isisd/isis_zebra.c b/isisd/isis_zebra.c index 45728ad2c..a75e293f9 100644 --- a/isisd/isis_zebra.c +++ b/isisd/isis_zebra.c @@ -570,7 +570,7 @@ isis_zebra_read_ipv4 (int command, struct zclient *zclient, api.message = stream_getc (stream); p.family = AF_INET; - p.prefixlen = stream_getc (stream); + p.prefixlen = MIN(IPV4_MAX_PREFIXLEN, stream_getc (stream)); stream_get (&p.prefix, stream, PSIZE (p.prefixlen)); if (CHECK_FLAG (api.message, ZAPI_MESSAGE_NEXTHOP)) diff --git a/lib/zclient.c b/lib/zclient.c index 5193a282a..6ccc6e371 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -942,18 +942,30 @@ zebra_redistribute_send (int command, struct zclient *zclient, afi_t afi, int ty return zclient_send_message(zclient); } +/* Get prefix in ZServ format; family should be filled in on prefix */ +static void +zclient_stream_get_prefix (struct stream *s, struct prefix *p) +{ + size_t plen = prefix_blen (p); + u_char c; + p->prefixlen = 0; + + if (plen == 0) + return; + + stream_get (&p->u.prefix, s, plen); + c = stream_getc(s); + p->prefixlen = MIN(plen * 8, c); +} + /* Router-id update from zebra daemon. */ void zebra_router_id_update_read (struct stream *s, struct prefix *rid) { - int plen; - /* Fetch interface address. */ rid->family = stream_getc (s); - - plen = prefix_blen (rid); - stream_get (&rid->u.prefix, s, plen); - rid->prefixlen = stream_getc (s); + + zclient_stream_get_prefix (s, rid); } /* Interface addition from zebra daemon. */ @@ -1263,8 +1275,7 @@ zebra_interface_address_read (int type, struct stream *s, vrf_id_t vrf_id) ifindex_t ifindex; struct interface *ifp; struct connected *ifc; - struct prefix p, d; - int family; + struct prefix p, d, *dp; int plen; u_char ifc_flags; @@ -1288,24 +1299,24 @@ zebra_interface_address_read (int type, struct stream *s, vrf_id_t vrf_id) ifc_flags = stream_getc (s); /* Fetch interface address. */ - family = p.family = stream_getc (s); - - plen = prefix_blen (&p); - stream_get (&p.u.prefix, s, plen); - p.prefixlen = stream_getc (s); + d.family = p.family = stream_getc (s); + plen = prefix_blen (&d); + + zclient_stream_get_prefix (s, &p); /* Fetch destination address. */ stream_get (&d.u.prefix, s, plen); - d.family = family; - + + /* N.B. NULL destination pointers are encoded as all zeroes */ + dp = memconstant(&d.u.prefix,0,plen) ? NULL : &d; + if (type == ZEBRA_INTERFACE_ADDRESS_ADD) { ifc = connected_lookup_prefix_exact (ifp, &p); if (!ifc) { /* N.B. NULL destination pointers are encoded as all zeroes */ - ifc = connected_add_by_prefix(ifp, &p, (memconstant(&d.u.prefix,0,plen) ? - NULL : &d)); + ifc = connected_add_by_prefix(ifp, &p, dp); } if (ifc) { diff --git a/ospf6d/ospf6_zebra.c b/ospf6d/ospf6_zebra.c index 6dee1424a..5969ef7b6 100644 --- a/ospf6d/ospf6_zebra.c +++ b/ospf6d/ospf6_zebra.c @@ -235,7 +235,7 @@ ospf6_zebra_read_ipv6 (int command, struct zclient *zclient, /* IPv6 prefix. */ memset (&p, 0, sizeof (struct prefix_ipv6)); p.family = AF_INET6; - p.prefixlen = stream_getc (s); + p.prefixlen = MIN(IPV6_MAX_PREFIXLEN, stream_getc (s)); stream_get (&p.prefix, s, PSIZE (p.prefixlen)); /* Nexthop, ifindex, distance, metric. */ diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c index 8752e83ed..0938ce8d1 100644 --- a/ospfd/ospf_zebra.c +++ b/ospfd/ospf_zebra.c @@ -1070,7 +1070,7 @@ ospf_zebra_read_ipv4 (int command, struct zclient *zclient, /* IPv4 prefix. */ memset (&p, 0, sizeof (struct prefix_ipv4)); p.family = AF_INET; - p.prefixlen = stream_getc (s); + p.prefixlen = MIN(IPV4_MAX_PREFIXLEN, stream_getc (s)); stream_get (&p.prefix, s, PSIZE (p.prefixlen)); if (IPV4_NET127(ntohl(p.prefix.s_addr))) diff --git a/ripd/rip_zebra.c b/ripd/rip_zebra.c index c312641d4..06e840464 100644 --- a/ripd/rip_zebra.c +++ b/ripd/rip_zebra.c @@ -153,7 +153,7 @@ rip_zebra_read_ipv4 (int command, struct zclient *zclient, zebra_size_t length, /* IPv4 prefix. */ memset (&p, 0, sizeof (struct prefix_ipv4)); p.family = AF_INET; - p.prefixlen = stream_getc (s); + p.prefixlen = MIN(IPV4_MAX_PREFIXLEN, stream_getc (s)); stream_get (&p.prefix, s, PSIZE (p.prefixlen)); /* Nexthop, ifindex, distance, metric. */ diff --git a/ripngd/ripng_zebra.c b/ripngd/ripng_zebra.c index 1184cd0db..e1ede095e 100644 --- a/ripngd/ripng_zebra.c +++ b/ripngd/ripng_zebra.c @@ -149,7 +149,7 @@ ripng_zebra_read_ipv6 (int command, struct zclient *zclient, /* IPv6 prefix. */ memset (&p, 0, sizeof (struct prefix_ipv6)); p.family = AF_INET6; - p.prefixlen = stream_getc (s); + p.prefixlen = MIN(IPV6_MAX_PREFIXLEN, stream_getc (s)); stream_get (&p.prefix, s, PSIZE (p.prefixlen)); /* Nexthop, ifindex, distance, metric. */ -- 2.39.5