From: Evan Hunt Date: Wed, 23 Mar 2016 15:54:46 +0000 (-0700) Subject: [master] fix ECS with family==0 X-Git-Tag: v9.11.0a1~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=395e6865d5b0ec13c1e4cc3947598153aa4e4914;p=thirdparty%2Fbind9.git [master] fix ECS with family==0 4341. [bug] Correct the handling of ECS options with address family 0. [RT #41377] --- diff --git a/CHANGES b/CHANGES index 98d58a5925d..a5d88d80ebc 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,8 @@ --- 9.11.0a1 released --- +4341. [bug] Correct the handling of ECS options with + address family 0. [RT #41377] + 4340. [performance] Implement adaptive read-write locks, reducing the overhead of locks that are only held briefly. [RT #37329] diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 665b1ce3700..a6ecf151148 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2527,14 +2527,14 @@ setup_lookup(dig_lookup_t *lookup) { if (sa->sa_family == AF_INET) { family = 1; sin = (struct sockaddr_in *) sa; - memcpy(addr, &sin->sin_addr, 4); + memmove(addr, &sin->sin_addr, 4); if ((plen % 8) != 0) addr[addrl-1] &= ~0 << (8 - (plen % 8)); } else { family = 2; sin6 = (struct sockaddr_in6 *) sa; - memcpy(addr, &sin6->sin6_addr, 16); + memmove(addr, &sin6->sin6_addr, 16); } /* Mask off last address byte */ diff --git a/bin/named/client.c b/bin/named/client.c index 1a25ee21f74..159ee8b9be8 100644 --- a/bin/named/client.c +++ b/bin/named/client.c @@ -1894,7 +1894,6 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) { isc_uint16_t family; isc_uint8_t addrlen, addrbytes, scope, *paddr; isc_netaddr_t caddr; - int i; /* * If we have already seen a ECS option skip this ECS option. @@ -1929,21 +1928,51 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) { return (DNS_R_OPTERR); } + if (addrlen == 0 && family != 0) { + ns_client_log(client, NS_LOGCATEGORY_CLIENT, + NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2), + "EDNS client-subnet option: " + "source == 0 but family != 0"); + return (DNS_R_OPTERR); + } + memset(&caddr, 0, sizeof(caddr)); switch (family) { + case 0: + /* + * XXXMUKS: In queries, if FAMILY is set to 0, SOURCE + * PREFIX-LENGTH must be 0 and ADDRESS should not be + * present as the address and prefix lengths don't make + * sense because the family is unknown. + */ + if (addrlen != 0U) { + ns_client_log(client, NS_LOGCATEGORY_CLIENT, + NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2), + "EDNS client-subnet option: invalid " + "address length (%u) for FAMILY=0", + addrlen); + return (DNS_R_OPTERR); + } + caddr.family = AF_UNSPEC; + break; case 1: - if (addrlen > 32U) - goto invalid_length; + if (addrlen > 32U) { + ns_client_log(client, NS_LOGCATEGORY_CLIENT, + NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2), + "EDNS client-subnet option: invalid " + "address length (%u) for IPv4", + addrlen); + return (DNS_R_OPTERR); + } caddr.family = AF_INET; break; case 2: if (addrlen > 128U) { - invalid_length: ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2), "EDNS client-subnet option: invalid " - "address length (%u) for %s", - addrlen, family == 1 ? "IPv4" : "IPv6"); + "address length (%u) for IPv6", + addrlen); return (DNS_R_OPTERR); } caddr.family = AF_INET6; @@ -1964,16 +1993,17 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) { } paddr = (isc_uint8_t *) &caddr.type; - for (i = 0; i < addrbytes; i++) { - paddr[i] = isc_buffer_getuint8(buf); - optlen--; - } - - if (addrbytes != 0U && (addrlen % 8) != 0) { - isc_uint8_t bits = ~0 << (8 - (addrlen % 8)); - bits &= paddr[addrbytes - 1]; - if (bits != paddr[addrbytes - 1]) - return (DNS_R_OPTERR); + if (addrbytes != 0U) { + memmove(paddr, isc_buffer_current(buf), addrbytes); + isc_buffer_forward(buf, addrbytes); + optlen -= addrbytes; + + if ((addrlen % 8) != 0) { + isc_uint8_t bits = ~0 << (8 - (addrlen % 8)); + bits &= paddr[addrbytes - 1]; + if (bits != paddr[addrbytes - 1]) + return (DNS_R_OPTERR); + } } memmove(&client->ecs_addr, &caddr, sizeof(caddr)); diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index 6e2d7d75007..d60182d5b05 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -611,14 +611,14 @@ sendquery(struct query *query, isc_task_t *task) if (sa->sa_family == AF_INET) { family = 1; sin = (struct sockaddr_in *) sa; - memcpy(addr, &sin->sin_addr, 4); + memmove(addr, &sin->sin_addr, 4); if ((plen % 8) != 0) addr[addrl-1] &= ~0 << (8 - (plen % 8)); } else { family = 2; sin6 = (struct sockaddr_in6 *) sa; - memcpy(addr, &sin6->sin6_addr, 16); + memmove(addr, &sin6->sin6_addr, 16); } /* Mask off last address byte */ diff --git a/lib/dns/message.c b/lib/dns/message.c index d05f218d873..0abd1671ca8 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -3244,6 +3244,9 @@ render_ecs(isc_buffer_t *ecsbuf, isc_buffer_t *target) { addrlen = isc_buffer_getuint8(ecsbuf); scopelen = isc_buffer_getuint8(ecsbuf); + if (addrlen == 0 && family != 0) + return (DNS_R_OPTERR); + addrbytes = (addrlen + 7) / 8; if (isc_buffer_remaininglength(ecsbuf) < addrbytes) return (DNS_R_OPTERR); @@ -3256,15 +3259,23 @@ render_ecs(isc_buffer_t *ecsbuf, isc_buffer_t *target) { for (i = 0; i < addrbytes; i ++) addr[i] = isc_buffer_getuint8(ecsbuf); - if (family == 1) { + switch (family) { + case 0: + if (addrlen != 0U || scopelen != 0U) + return (DNS_R_OPTERR); + strlcpy(addr_text, "0", sizeof(addr_text)); + break; + case 1: if (addrlen > 32 || scopelen > 32) return (DNS_R_OPTERR); inet_ntop(AF_INET, addr, addr_text, sizeof(addr_text)); - } else if (family == 2) { + break; + case 2: if (addrlen > 128 || scopelen > 128) return (DNS_R_OPTERR); inet_ntop(AF_INET6, addr, addr_text, sizeof(addr_text)); - } else { + break; + default: snprintf(addr_text, sizeof(addr_text), "Unsupported family %u", family); ADD_STRING(target, addr_text); diff --git a/lib/dns/rdata/generic/opt_41.c b/lib/dns/rdata/generic/opt_41.c index bc2de379088..55e3718e768 100644 --- a/lib/dns/rdata/generic/opt_41.c +++ b/lib/dns/rdata/generic/opt_41.c @@ -15,8 +15,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id$ */ - /* Reviewed: Thu Mar 16 14:06:44 PST 2000 by gson */ /* RFC2671 */ @@ -135,7 +133,24 @@ fromwire_opt(ARGS_FROMWIRE) { isc_region_consume(&sregion, 1); scope = uint8_fromregion(&sregion); isc_region_consume(&sregion, 1); + + if (addrlen == 0U && family != 0U) + return (DNS_R_OPTERR); + switch (family) { + case 0: + /* + * XXXMUKS: In queries and replies, if + * FAMILY is set to 0, SOURCE + * PREFIX-LENGTH and SCOPE PREFIX-LENGTH + * must be 0 and ADDRESS should not be + * present as the address and prefix + * lengths don't make sense because the + * family is unknown. + */ + if (addrlen != 0U || scope != 0U) + return (DNS_R_OPTERR); + break; case 1: if (addrlen > 32U || scope > 32U) return (DNS_R_OPTERR);