[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]

diff --git a/CHANGES b/CHANGES
index 2e6f9b19d7faad61fc953c023efbf9d3d69ee7c8..f17f871531d57d177b1e201f1e0d5229d938c54e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -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]

diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml
index 133097231db2c33d856800119d0a3103ecba893f..a2d5f7852c334a24b0a9efbe61a8cb9f431c0b40 100644 (file)
--- a/doc/arm/notes.xml
+++ b/doc/arm/notes.xml
@@ -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

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index e5899d0a26f45d5e9c4b618311bf6ea9af619da6..77312a85b0fd419a44c197df8c9c9b75352449f3 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -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);
 

diff --git a/lib/dns/validator.c b/lib/dns/validator.c
index db9d4baf0dcefe6a7b672be17d150ee25e2169f2..463edb1ce2436be939daa69cb6ba882eb35b51b1 100644 (file)
--- a/lib/dns/validator.c
+++ b/lib/dns/validator.c
@@ -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