Modify the DDNS handling code. In a previous patch we added logging

code to the DDNS handling.  This code included a bug that caused it
to attempt to dereference a NULL pointer and eventually segfault.
While reviewing the code as we addressed this problem, we determined
that some of the updates to the lease structures would not work as
planned since the structures being updated were in the process of
being freed: these updates were removed.  In addition we removed an
incorrect call to the DDNS removal function that could cause a failure
during the removal of DDNS information from the DNS server.
Thanks to Jasper Jongmans for reporting this issue.
[ISC-Bugs #27078]
CVE: CVE-2011-4868

diff --git a/RELNOTES b/RELNOTES
index eb5bd2a615cc3d43467e6f7546d5c4f1bc8cf6de..f1a50e6b45f948c1103109f569585aee3800993f 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -48,6 +48,19 @@ work on other platforms. Please report any problems and suggested fixes to
   [ISC-Bugs #26704].
   CVE: CVE-2011-4539
 
+! Modify the DDNS handling code.  In a previous patch we added logging
+  code to the DDNS handling.  This code included a bug that caused it
+  to attempt to dereference a NULL pointer and eventually segfault.
+  While reviewing the code as we addressed this problem, we determined
+  that some of the updates to the lease structures would not work as
+  planned since the structures being updated were in the process of
+  being freed: these updates were removed.  In addition we removed an
+  incorrect call to the DDNS removal function that could cause a failure
+  during the removal of DDNS information from the DNS server.
+  Thanks to Jasper Jongmans for reporting this issue.
+  [ISC-Bugs #27078]
+  CVE: CVE-2011-4868
+
                        Changes since 4.2.2
 
 - Fix the code that checks for an existing DDNS transaction to cancel

diff --git a/includes/dhcpd.h b/includes/dhcpd.h
index a4d81f34b39061b06cc3ea94b0c9df17c5569102..566e1e30cd03747ab5c96b18b5a6c1d61751a6c0 100644 (file)
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -1539,7 +1539,7 @@ struct ipv6_pool {
 #define DDNS_EXECUTE_NEXT       0x20
 #define DDNS_ABORT              0x40
 #define DDNS_STATIC_LEASE       0x80
-
+#define DDNS_ACTIVE_LEASE      0x100
 /*
  * The following two groups are separate and we could reuse
  * values but not reusing them may be useful in the future.
@@ -1580,7 +1580,7 @@ typedef struct dhcp_ddns_cb {
        int zone_addr_count;
        struct dns_zone *zone;
 
-       int flags;
+       u_int16_t flags;
        TIME timeout;
        int state;
        ddns_action_t cur_func;
@@ -1932,7 +1932,7 @@ void parse_server_duid_conf(struct parse *cfile);
 /* ddns.c */
 int ddns_updates(struct packet *, struct lease *, struct lease *,
                 struct iasubopt *, struct iasubopt *, struct option_state *);
-int ddns_removals(struct lease *, struct iasubopt *, struct dhcp_ddns_cb *);
+int ddns_removals(struct lease *, struct iasubopt *, struct dhcp_ddns_cb *, isc_boolean_t);
 #if defined (TRACING)
 void trace_ddns_init(void);
 #endif

diff --git a/server/ddns.c b/server/ddns.c
index b003bd0b1f81fbd71ddc0230cb6b95f66880eb08..d92a7ef1b4ae08690c9f9b7ee40cb55d2c9f1a44 100644 (file)
--- a/server/ddns.c
+++ b/server/ddns.c
@@ -124,8 +124,12 @@ ddns_updates(struct packet *packet, struct lease *lease, struct lease *old,
        if (ddns_cb == NULL) {
                return(0);
        }
-       /* assume that we shall update both the A and ptr records */
-       ddns_cb->flags = DDNS_UPDATE_ADDR | DDNS_UPDATE_PTR;
+       /*
+        * Assume that we shall update both the A and ptr records and,
+        * as this is an update, set the active flag 
+        */
+       ddns_cb->flags = DDNS_UPDATE_ADDR | DDNS_UPDATE_PTR |
+               DDNS_ACTIVE_LEASE;
 
        /*
         * For v4 we flag static leases so we don't try
@@ -330,7 +334,7 @@ ddns_updates(struct packet *packet, struct lease *lease, struct lease *old,
 
                /* If desired do the removals */
                if (do_remove != 0) {
-                       (void) ddns_removals(lease, lease6, NULL);
+                       (void) ddns_removals(lease, lease6, NULL, ISC_TRUE);
                }
                goto out;
        }
@@ -532,7 +536,7 @@ ddns_updates(struct packet *packet, struct lease *lease, struct lease *old,
         * the ddns messages.  Currently we don't.
         */
        if (do_remove) {
-               rcode1 = ddns_removals(lease, lease6, ddns_cb);
+               rcode1 = ddns_removals(lease, lease6, ddns_cb, ISC_TRUE);
        }
        else {
                ddns_fwd_srv_connector(lease, lease6, scope, ddns_cb,
@@ -740,6 +744,15 @@ ddns_update_lease_text(dhcp_ddns_cb_t        *ddns_cb,
        if (ddns_cb->flags & DDNS_STATIC_LEASE)
                return (ISC_R_SUCCESS);
 
+       /* 
+        * If we are processing an expired or released v6 lease
+        * we don't actually have a scope to update
+        */
+       if ((ddns_cb->address.len == 16) &&
+           ((ddns_cb->flags & DDNS_ACTIVE_LEASE) == 0)) {
+               return (ISC_R_SUCCESS);
+       }
+
        if (inscope != NULL) {
                scope = inscope;
        } else if (ddns_cb->address.len == 4) {
@@ -1085,6 +1098,15 @@ ddns_update_lease_ptr(struct lease    *lease,
                return (ISC_R_SUCCESS);
        }
 
+       /* 
+        * If we are processing an expired or released v6 lease
+        * we don't actually have a scope to update
+        */
+       if ((ddns_cb->address.len == 16) &&
+           ((ddns_cb->flags & DDNS_ACTIVE_LEASE) == 0)) {
+               return (ISC_R_SUCCESS);
+       }
+
        if (lease != NULL) {
                safe_lease_update(lease, ddns_cb, ddns_cb_set, 
                                  file, line);
@@ -1132,7 +1154,7 @@ ddns_update_lease_ptr(struct lease    *lease,
                     ISC_R_SUCCESS) &&
                    (find_ipv6_pool(&pool, D6O_IA_NA, &addr) != 
                     ISC_R_SUCCESS)) {
-                       inet_ntop(AF_INET6, &lease6->addr, addrbuf,
+                       inet_ntop(AF_INET6, &addr, addrbuf,
                                  MAX_ADDRESS_STRING_LEN);
                        log_error("%s(%d): Pool for lease %s not found.",
                                  file, line, addrbuf);
@@ -1152,7 +1174,7 @@ ddns_update_lease_ptr(struct lease    *lease,
                        find_lease6->ddns_cb = ddns_cb_set;
                        iasubopt_dereference(&find_lease6, MDL);
                } else {
-                       inet_ntop(AF_INET6, &lease6->addr, addrbuf,
+                       inet_ntop(AF_INET6, &addr, addrbuf,
                                  MAX_ADDRESS_STRING_LEN);
                        log_error("%s(%d): Lease %s not found within pool.",
                                  file, line, addrbuf);
@@ -1594,12 +1616,18 @@ ddns_fwd_srv_rem1(dhcp_ddns_cb_t *ddns_cb,
  * 0 - badness occurred and we weren't able to do what was wanted
  * 1 - we were able to do stuff but it's in progress
  * in both cases any additional block has been passed on to it's handler
+ * 
+ * active == ISC_TRUE if the lease is still active, and FALSE if the lease
+ * is inactive.  This is used to indicate if the lease is inactive or going
+ * to inactive in IPv6 so we can avoid trying to update the lease with
+ * cb pointers and text information.
  */
 
 int
 ddns_removals(struct lease    *lease,
              struct iasubopt *lease6,
-             dhcp_ddns_cb_t  *add_ddns_cb)
+             dhcp_ddns_cb_t  *add_ddns_cb,
+             isc_boolean_t    active)
 {
        isc_result_t rcode, execute_add = ISC_R_FAILURE;
        struct binding_scope **scope = NULL;
@@ -1644,6 +1672,16 @@ ddns_removals(struct lease    *lease,
        } else
                goto cleanup;
 
+       /*
+        * Set the flag bit if the lease is active, that is it isn't
+        * expired or released.  This is used in the IPv6 paths to
+        * determine if we need to update the lease when the response
+        * from the DNS code is processed.
+        */
+       if (active == ISC_TRUE) {
+               ddns_cb->flags |= DDNS_ACTIVE_LEASE;
+       }
+
        /* No scope implies that DDNS has not been performed for this lease. */
        if (*scope == NULL)
                goto cleanup;

diff --git a/server/failover.c b/server/failover.c
index 97e7d734adf27a92c726f82a61b7571b107d96e1..26be290cc0036006cd9d31b92acd3439ac72bebd 100644 (file)
--- a/server/failover.c
+++ b/server/failover.c
@@ -5218,7 +5218,7 @@ isc_result_t dhcp_failover_process_bind_update (dhcp_failover_state_t *state,
         */
        if (msg->binding_status == FTS_ACTIVE &&
            (chaddr_changed || ident_changed)) {
-               ddns_removals(lease, NULL, NULL);
+               ddns_removals(lease, NULL, NULL, ISC_FALSE);
 
                if (lease->scope != NULL)
                        binding_scope_dereference(&lease->scope, MDL);

diff --git a/server/mdb.c b/server/mdb.c
index 53c1b3091750c2c48315abd8587595cbbaa7efcd..cdf4ff1bca17609cf39f2699ba241c55df269c1f 100644 (file)
--- a/server/mdb.c
+++ b/server/mdb.c
@@ -1446,7 +1446,7 @@ void make_binding_state_transition (struct lease *lease)
              lease -> binding_state == FTS_ACTIVE &&
              lease -> next_binding_state != FTS_RELEASED))) {
 #if defined (NSUPDATE)
-               ddns_removals(lease, NULL, NULL);
+               ddns_removals(lease, NULL, NULL, ISC_FALSE);
 #endif
                if (lease -> on_expiry) {
                        execute_statements ((struct binding_value **)0,
@@ -1512,7 +1512,7 @@ void make_binding_state_transition (struct lease *lease)
                 * release message.  This is not true of expiry, where the
                 * peer may have extended the lease.
                 */
-               ddns_removals(lease, NULL, NULL);
+               ddns_removals(lease, NULL, NULL, ISC_FALSE);
 #endif
                if (lease -> on_release) {
                        execute_statements ((struct binding_value **)0,
@@ -1681,7 +1681,7 @@ void release_lease (lease, packet)
        /* If there are statements to execute when the lease is
           released, execute them. */
 #if defined (NSUPDATE)
-       ddns_removals(lease, NULL, NULL);
+       ddns_removals(lease, NULL, NULL, ISC_FALSE);
 #endif
        if (lease -> on_release) {
                execute_statements ((struct binding_value **)0,
@@ -1755,7 +1755,7 @@ void abandon_lease (lease, message)
 {
        struct lease *lt = (struct lease *)0;
 #if defined (NSUPDATE)
-       ddns_removals(lease, NULL, NULL);
+       ddns_removals(lease, NULL, NULL, ISC_FALSE);
 #endif
 
        if (!lease_copy (&lt, lease, MDL))
@@ -1787,7 +1787,7 @@ void dissociate_lease (lease)
 {
        struct lease *lt = (struct lease *)0;
 #if defined (NSUPDATE)
-       ddns_removals(lease, NULL, NULL);
+       ddns_removals(lease, NULL, NULL, ISC_FALSE);
 #endif
 
        if (!lease_copy (&lt, lease, MDL))

diff --git a/server/mdb6.c b/server/mdb6.c
index 9d410f551ff64abad96de287e529c5e240f1f649..1925be00a16d9119a0ed863b14a10d6ad0696d62 100644 (file)
--- a/server/mdb6.c
+++ b/server/mdb6.c
@@ -1058,7 +1058,7 @@ move_lease_to_inactive(struct ipv6_pool *pool, struct iasubopt *lease,
 #if defined (NSUPDATE)
                /* Process events upon expiration. */
                if (pool->pool_type != D6O_IA_PD) {
-                       ddns_removals(NULL, lease, NULL);
+                       ddns_removals(NULL, lease, NULL, ISC_FALSE);
                }
 #endif
 
@@ -1466,6 +1466,11 @@ lease_timeout_support(void *vpool) {
                 * Note that if there are no leases in the pool, 
                 * expire_lease6() will return ISC_R_SUCCESS with 
                 * a NULL lease.
+                *
+                * expire_lease6() will call move_lease_to_inactive() which
+                * calls ddns_removals() do we want that on the standard
+                * expiration timer or a special 'depref' timer?  Original
+                * query from DH, moved here by SAR.
                 */
                lease = NULL;
                if (expire_lease6(&lease, pool, cur_time) != ISC_R_SUCCESS) {
@@ -1475,18 +1480,6 @@ lease_timeout_support(void *vpool) {
                        break;
                }
 
-               /* Look to see if there were ddns updates, and if
-                * so, drop them.
-                *
-                * DH: Do we want to do this on a special 'depref'
-                * timer rather than expiration timer?
-                */
-#if defined (NSUPDATE)
-               if (pool->pool_type != D6O_IA_PD) {
-                       ddns_removals(NULL, lease, NULL);
-               }
-#endif
-
                write_ia(lease->ia);
 
                iasubopt_dereference(&lease, MDL);