[v9_12_0_patch] block validator deadlock and prevent use-after-free

4859.	[bug]		A loop was possible when attempting to validate
			unsigned CNAME responses from secure zones;
			this caused a delay in returning SERVFAIL and
			also increased the chances of encountering
			CVE-2017-3145. [RT #46839]

4858.	[security]	Addresses could be referenced after being freed
			in resolver.c, causing an assertion failure.
			(CVE-2017-3145) [RT #46839]
This commit is contained in:
Evan Hunt
2018-01-03 19:18:08 -08:00
parent 1b8a33b7e0
commit d6376468bf
4 changed files with 102 additions and 18 deletions

12
CHANGES
View File

@@ -1,4 +1,14 @@
--- 9.12.0 released ---
--- 9.12.0rc2 released ---
4859. [bug] A loop was possible when attempting to validate
unsigned CNAME responses from secure zones;
this caused a delay in returning SERVFAIL and
also increased the chances of encountering
CVE-2017-3145. [RT #46839]
4858. [security] Addresses could be referenced after being freed
in resolver.c, causing an assertion failure.
(CVE-2017-3145) [RT #46839]
4850. [bug] Named failed to restart with multiple added zones in
lmdb database. [RT #46889]

View File

@@ -21,7 +21,9 @@
BIND 9.12.0 is a new feature release of BIND. This document
summarizes new features and functional changes that have been
introduced on this branch, as well as features that have been
deprecated or removed.
deprecated or removed. With each development release leading
up to the final BIND 9.12.0 release, this document will be
updated with additional features added and bugs fixed.
</para>
</section>
@@ -35,6 +37,20 @@
</para>
</section>
<section xml:id="relnotes_security"><info><title>Security Fixes</title></info>
<itemizedlist>
<listitem>
<para>
Addresses could be referenced after being freed during resolver
processing, causing an assertion failure. The chances of this
happening were remote, but the introduction of a delay in
resolution increasred them. This bug is disclosed in
CVE-2017-3145. [RT #46839]
</para>
</listitem>
</itemizedlist>
</section>
<section xml:id="relnotes_features"><info><title>New Features</title></info>
<itemizedlist>
<listitem>
@@ -676,6 +692,54 @@
</itemizedlist>
</section>
<section xml:id="relnotes_bugs"><info><title>Bug Fixes</title></info>
<itemizedlist>
<listitem>
<para>
Attempting to validate improperly unsigned CNAME responses
from secure zones could cause a validator loop. This caused
a delay in returning SERVFAIL and also increased the chances
of encountering the crash bug described in CVE-2017-3145.
[RT #46839]
</para>
</listitem>
<listitem>
<para>
Zones created with <command>rndc addzone</command> could
temporarily fail to inherit the <command>allow-transfer</command>
ACL set in the <command>options</command> section of
<filename>named.conf</filename>. [RT #46603]
</para>
</listitem>
<listitem>
<para>
The introduction of <command>libns</command> caused a bug
in which TCP client objects were not recycled after use,
leading to unconstrained memory growth. [RT #46029]
</para>
</listitem>
<listitem>
<para>
Some header files included &lt;isc/util.h&gt; incorrectly as
it pollutes with namespace with non ISC_ macros and this should
only be done by explicitly including &lt;isc/util.h&gt;. This
has been corrected. Some code may depend on &lt;isc/util.h&gt;
being implicitly included via other header files. Such
code should explicitly include &lt;isc/util.h&gt;.
</para>
</listitem>
<listitem>
<para>
<command>named</command> failed to properly determine whether
there were active KSK and ZSK keys for an algorithm when
<command>update-check-ksk</command> was true (which is the
default setting). This could leave records unsigned
when rolling keys. [RT #46743] [RT #46754] [RT #46774]
</para>
</listitem>
</itemizedlist>
</section>
<section xml:id="relnotes_license"><info><title>License</title></info>
<para>
BIND is open source software licenced under the terms of the Mozilla

View File

@@ -1051,7 +1051,7 @@ fctx_stoptimer(fetchctx_t *fctx) {
* cannot fail in that case.
*/
result = isc_timer_reset(fctx->timer, isc_timertype_inactive,
NULL, NULL, ISC_TRUE);
NULL, NULL, ISC_TRUE);
if (result != ISC_R_SUCCESS) {
UNEXPECTED_ERROR(__FILE__, __LINE__,
"isc_timer_reset(): %s",
@@ -1059,7 +1059,6 @@ fctx_stoptimer(fetchctx_t *fctx) {
}
}
static inline isc_result_t
fctx_startidletimer(fetchctx_t *fctx, isc_interval_t *interval) {
/*
@@ -1336,7 +1335,8 @@ fctx_cleanupfinds(fetchctx_t *fctx) {
for (find = ISC_LIST_HEAD(fctx->finds);
find != NULL;
find = next_find) {
find = next_find)
{
next_find = ISC_LIST_NEXT(find, publink);
ISC_LIST_UNLINK(fctx->finds, find, publink);
dns_adb_destroyfind(&find);
@@ -1352,7 +1352,8 @@ fctx_cleanupaltfinds(fetchctx_t *fctx) {
for (find = ISC_LIST_HEAD(fctx->altfinds);
find != NULL;
find = next_find) {
find = next_find)
{
next_find = ISC_LIST_NEXT(find, publink);
ISC_LIST_UNLINK(fctx->altfinds, find, publink);
dns_adb_destroyfind(&find);
@@ -1368,7 +1369,8 @@ fctx_cleanupforwaddrs(fetchctx_t *fctx) {
for (addr = ISC_LIST_HEAD(fctx->forwaddrs);
addr != NULL;
addr = next_addr) {
addr = next_addr)
{
next_addr = ISC_LIST_NEXT(addr, publink);
ISC_LIST_UNLINK(fctx->forwaddrs, addr, publink);
dns_adb_freeaddrinfo(fctx->adb, &addr);
@@ -1383,7 +1385,8 @@ fctx_cleanupaltaddrs(fetchctx_t *fctx) {
for (addr = ISC_LIST_HEAD(fctx->altaddrs);
addr != NULL;
addr = next_addr) {
addr = next_addr)
{
next_addr = ISC_LIST_NEXT(addr, publink);
ISC_LIST_UNLINK(fctx->altaddrs, addr, publink);
dns_adb_freeaddrinfo(fctx->adb, &addr);
@@ -1391,16 +1394,20 @@ fctx_cleanupaltaddrs(fetchctx_t *fctx) {
}
static inline void
fctx_stopeverything(fetchctx_t *fctx, isc_boolean_t no_response,
isc_boolean_t age_untried)
fctx_stopqueries(fetchctx_t *fctx, isc_boolean_t no_response,
isc_boolean_t age_untried)
{
FCTXTRACE("stopeverything");
FCTXTRACE("stopqueries");
fctx_cancelqueries(fctx, no_response, age_untried);
fctx_stoptimer(fctx);
}
static inline void
fctx_cleanupall(fetchctx_t *fctx) {
fctx_cleanupfinds(fctx);
fctx_cleanupaltfinds(fctx);
fctx_cleanupforwaddrs(fctx);
fctx_cleanupaltaddrs(fctx);
fctx_stoptimer(fctx);
}
static void
@@ -1651,7 +1658,8 @@ fctx_done(fetchctx_t *fctx, isc_result_t result, int line) {
age_untried = ISC_TRUE;
fctx->reason = NULL;
fctx_stopeverything(fctx, no_response, age_untried);
fctx_stopqueries(fctx, no_response, age_untried);
LOCK(&res->buckets[fctx->bucketnum].lock);
@@ -4271,11 +4279,12 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) {
dns_resolver_cancelfetch(fctx->nsfetch);
/*
* Shut down anything that is still running on behalf of this
* fetch. To avoid deadlock with the ADB, we must do this
* before we lock the bucket lock.
* Shut down anything still running on behalf of this
* fetch, and clean up finds and addresses. To avoid deadlock
* with the ADB, we must do this before we lock the bucket lock.
*/
fctx_stopeverything(fctx, ISC_FALSE, ISC_FALSE);
fctx_stopqueries(fctx, ISC_FALSE, ISC_FALSE);
fctx_cleanupall(fctx);
LOCK(&res->buckets[bucketnum].lock);

View File

@@ -1100,7 +1100,8 @@ check_deadlock(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type,
for (parent = val; parent != NULL; parent = parent->parent) {
if (parent->event != NULL &&
parent->event->type == type &&
(parent->event->type == type ||
parent->event->type == dns_rdatatype_cname) &&
dns_name_equal(parent->event->name, name) &&
/*
* As NSEC3 records are meta data you sometimes