From 455c0848f80a8acda27aad1466c72987cafaa029 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Sat, 27 Feb 2016 11:23:50 +1100 Subject: [PATCH] 4322. [security] Duplicate EDNS COOKIE options in a response could trigger an assertion failure. (CVE-2016-2088) [RT #41809] --- CHANGES | 4 ++++ bin/dig/dighost.c | 9 +++++++++ bin/named/client.c | 21 ++++++++++++++++++--- doc/arm/notes.xml | 7 +++++++ lib/dns/resolver.c | 14 +++++++++++++- 5 files changed, 51 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index e26cad63c2..d4344c0004 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +4322. [security] Duplicate EDNS COOKIE options in a response could + trigger an assertion failure. (CVE-2016-2088) + [RT #41809] + 4321. [bug] Zones using mapped files containing out-of-zone data could return SERVFAIL instead of the expected NODATA or NXDOMAIN results. [RT #41596] diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 96b44a84af..665b1ce370 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3536,6 +3536,7 @@ process_opt(dig_lookup_t *l, dns_message_t *msg) { isc_buffer_t optbuf; isc_uint16_t optcode, optlen; dns_rdataset_t *opt = msg->opt; + isc_boolean_t seen_cookie = ISC_FALSE; result = dns_rdataset_first(opt); if (result == ISC_R_SUCCESS) { @@ -3548,7 +3549,15 @@ process_opt(dig_lookup_t *l, dns_message_t *msg) { optlen = isc_buffer_getuint16(&optbuf); switch (optcode) { case DNS_OPT_COOKIE: + /* + * Only process the first cookie option. + */ + if (seen_cookie) { + isc_buffer_forward(&optbuf, optlen); + break; + } process_cookie(l, msg, &optbuf, optlen); + seen_cookie = ISC_TRUE; break; default: isc_buffer_forward(&optbuf, optlen); diff --git a/bin/named/client.c b/bin/named/client.c index 21dabd2d28..1a25ee21f7 100644 --- a/bin/named/client.c +++ b/bin/named/client.c @@ -122,6 +122,9 @@ #define COOKIE_SIZE 24U /* 8 + 4 + 4 + 8 */ #define ECS_SIZE 20U /* 2 + 1 + 1 + [0..16] */ +#define WANTNSID(x) (((x)->attributes & NS_CLIENTATTR_WANTNSID) != 0) +#define WANTEXPIRE(x) (((x)->attributes & NS_CLIENTATTR_WANTEXPIRE) != 0) + /*% nameserver client manager structure */ struct ns_clientmgr { /* Unlocked. */ @@ -1516,7 +1519,7 @@ ns_client_addopt(ns_client_t *client, dns_message_t *message, flags = client->extflags & DNS_MESSAGEEXTFLAG_REPLYPRESERVE; /* Set EDNS options if applicable */ - if ((client->attributes & NS_CLIENTATTR_WANTNSID) != 0 && + if (WANTNSID(client) && (ns_g_server->server_id != NULL || ns_g_server->server_usehostname)) { if (ns_g_server->server_usehostname) { @@ -1893,6 +1896,14 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) { isc_netaddr_t caddr; int i; + /* + * If we have already seen a ECS option skip this ECS option. + */ + if ((client->attributes & NS_CLIENTATTR_HAVEECS) != 0) { + isc_buffer_forward(buf, optlen); + return (ISC_R_SUCCESS); + } + /* * XXXMUKS: Is there any need to repeat these checks here * (except query's scope length) when they are done in the OPT @@ -2028,7 +2039,9 @@ process_opt(ns_client_t *client, dns_rdataset_t *opt) { optlen = isc_buffer_getuint16(&optbuf); switch (optcode) { case DNS_OPT_NSID: - isc_stats_increment(ns_g_server->nsstats, + if (!WANTNSID(client)) + isc_stats_increment( + ns_g_server->nsstats, dns_nsstatscounter_nsidopt); client->attributes |= NS_CLIENTATTR_WANTNSID; isc_buffer_forward(&optbuf, optlen); @@ -2037,7 +2050,9 @@ process_opt(ns_client_t *client, dns_rdataset_t *opt) { process_cookie(client, &optbuf, optlen); break; case DNS_OPT_EXPIRE: - isc_stats_increment(ns_g_server->nsstats, + if (!WANTEXPIRE(client)) + isc_stats_increment( + ns_g_server->nsstats, dns_nsstatscounter_expireopt); client->attributes |= NS_CLIENTATTR_WANTEXPIRE; isc_buffer_forward(&optbuf, optlen); diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml index cd472f5468..66b27e8366 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -42,6 +42,13 @@
Security Fixes + + + Duplicate EDNS COOKIE options in a response could trigger + an assertion failure. This flaw is disclosed in CVE-2016-2088. + [RT #41809] + + Insufficient testing when parsing a message allowed diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 991b2e78d1..c0e887c90b 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -7560,6 +7560,8 @@ process_opt(resquery_t *query, dns_rdataset_t *opt) { unsigned char *optvalue; dns_adbaddrinfo_t *addrinfo; unsigned char cookie[8]; + isc_boolean_t seen_cookie = ISC_FALSE; + isc_boolean_t seen_nsid = ISC_FALSE; result = dns_rdataset_first(opt); if (result == ISC_R_SUCCESS) { @@ -7573,13 +7575,22 @@ process_opt(resquery_t *query, dns_rdataset_t *opt) { INSIST(optlen <= isc_buffer_remaininglength(&optbuf)); switch (optcode) { case DNS_OPT_NSID: - if (query->options & DNS_FETCHOPT_WANTNSID) + if (!seen_nsid && + query->options & DNS_FETCHOPT_WANTNSID) log_nsid(&optbuf, optlen, query, ISC_LOG_DEBUG(3), query->fctx->res->mctx); isc_buffer_forward(&optbuf, optlen); + seen_nsid = ISC_TRUE; break; case DNS_OPT_COOKIE: + /* + * Only process the first cookie option. + */ + if (seen_cookie) { + isc_buffer_forward(&optbuf, optlen); + break; + } optvalue = isc_buffer_current(&optbuf); compute_cc(query, cookie, sizeof(cookie)); INSIST(query->fctx->rmessage->cc_bad == 0 && @@ -7598,6 +7609,7 @@ process_opt(resquery_t *query, dns_rdataset_t *opt) { isc_buffer_forward(&optbuf, optlen); inc_stats(query->fctx->res, dns_resstatscounter_cookiein); + seen_cookie = ISC_TRUE; break; default: isc_buffer_forward(&optbuf, optlen);