diff --git a/RELNOTES b/RELNOTES index eb5bd2a6..f1a50e6b 100644 --- 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 a4d81f34..566e1e30 100644 --- 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 b003bd0b..d92a7ef1 100644 --- 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 97e7d734..26be290c 100644 --- 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 53c1b309..cdf4ff1b 100644 --- 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 (<, 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 (<, lease, MDL)) diff --git a/server/mdb6.c b/server/mdb6.c index 9d410f55..1925be00 100644 --- 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);