2
0
mirror of https://gitlab.isc.org/isc-projects/dhcp synced 2025-08-22 09:57:20 +00:00

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
This commit is contained in:
Shawn Routhier 2011-12-30 23:08:41 +00:00
parent c535de4411
commit 0ef9a46e33
6 changed files with 73 additions and 29 deletions

View File

@ -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

View File

@ -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

View File

@ -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;

View File

@ -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);

View File

@ -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))

View File

@ -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);