upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-04-22 06:37:42 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

[9.20] fix: usr: Fix write after free in validator code

Browse source 
Raw integer pointers were being used for the validator's nvalidations
and nfails values but the memory holding them could be freed before
they ceased to be used.  Use reference counted counters instead.

Closes #5239

Backport of MR !10248

Merge branch 'backport-5239-use-counter-for-nvalidations-and-nfailss-9.20' into 'bind-9.20'

See merge request isc-projects/bind9!10300
This commit is contained in:
Mark Andrews 2025-03-20 03:45:07 +00:00
commit 5de1b3ba3c
3 changed files with 53 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
lib/dns/include/dns/validator.h
View file

@ -153,8 +153,8 @@ struct dns_validator {
uint8_t unsupported_digest;
dns_rdata_t rdata;
bool resume;
uint32_t *nvalidations;
uint32_t *nfails;
isc_counter_t *nvalidations;
isc_counter_t *nfails;
isc_counter_t *qc;
isc_counter_t *gqc;
@ -176,7 +176,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset,
dns_message_t *message, unsigned int options,
isc_loop_t *loop, isc_job_cb cb, void *arg,
uint32_t *nvalidations, uint32_t *nfails,
isc_counter_t *nvalidations, isc_counter_t *nfails,
isc_counter_t *qc, isc_counter_t *gqc,
dns_edectx_t *edectx, dns_validator_t **validatorp);
/*%<

32
lib/dns/resolver.c
View file

@ -480,8 +480,8 @@ struct fetchctx {
unsigned int depth;
char clientstr[ISC_SOCKADDR_FORMATSIZE];
uint32_t nvalidations;
uint32_t nfails;
isc_counter_t *nvalidations;
isc_counter_t *nfails;
};
#define FCTX_MAGIC ISC_MAGIC('F', '!', '!', '!')
@ -986,8 +986,8 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo,
result = dns_validator_create(
fctx->res->view, name, type, rdataset, sigrdataset, message,
valoptions, fctx->loop, validated, valarg, &fctx->nvalidations,
&fctx->nfails, fctx->qc, fctx->gqc, &fctx->edectx, &validator);
valoptions, fctx->loop, validated, valarg, fctx->nvalidations,
fctx->nfails, fctx->qc, fctx->gqc, &fctx->edectx, &validator);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
inc_stats(fctx->res, dns_resstatscounter_val);
if ((valoptions & DNS_VALIDATOR_DEFER) == 0) {
@ -4392,6 +4392,12 @@ fctx_destroy(fetchctx_t *fctx) {
isc_mem_put(fctx->mctx, sa, sizeof(*sa));
}
if (fctx->nfails != NULL) {
isc_counter_detach(&fctx->nfails);
}
if (fctx->nvalidations != NULL) {
isc_counter_detach(&fctx->nvalidations);
}
isc_counter_detach(&fctx->qc);
if (fctx->gqc != NULL) {
isc_counter_detach(&fctx->gqc);
@ -4561,6 +4567,8 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name,
char buf[DNS_NAME_FORMATSIZE + DNS_RDATATYPE_FORMATSIZE + 1];
isc_mem_t *mctx = isc_loop_getmctx(loop);
size_t p;
uint32_t nvalidations = atomic_load_relaxed(&res->maxvalidations);
uint32_t nfails = atomic_load_relaxed(&res->maxvalidationfails);
/*
* Caller must be holding the lock for 'bucket'
@ -4579,8 +4587,6 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name,
.fwdpolicy = dns_fwdpolicy_none,
.result = ISC_R_FAILURE,
.loop = loop,
.nvalidations = atomic_load_relaxed(&res->maxvalidations),
.nfails = atomic_load_relaxed(&res->maxvalidationfails),
};
isc_mem_attach(mctx, &fctx->mctx);
@ -4602,6 +4608,14 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name,
FCTXTRACE("create");
if (nfails > 0) {
isc_counter_create(mctx, nfails, &fctx->nfails);
}
if (nvalidations > 0) {
isc_counter_create(mctx, nvalidations, &fctx->nvalidations);
}
if (qc != NULL) {
isc_counter_attach(qc, &fctx->qc);
isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER,
@ -4834,6 +4848,12 @@ cleanup_nameservers:
dns_rdataset_disassociate(&fctx->nameservers);
}
isc_mem_free(fctx->mctx, fctx->info);
if (fctx->nfails != NULL) {
isc_counter_detach(&fctx->nfails);
}
if (fctx->nvalidations != NULL) {
isc_counter_detach(&fctx->nvalidations);
}
isc_counter_detach(&fctx->qc);
if (fctx->gqc != NULL) {
isc_counter_detach(&fctx->gqc);

35
lib/dns/validator.c
View file

@ -1256,7 +1256,10 @@ compute_keytag(dns_rdata_t *rdata) {
static bool
over_max_validations(dns_validator_t *val) {
if (val->nvalidations == NULL || (*val->nvalidations) > 0) {
if (val->nvalidations == NULL ||
isc_counter_used(val->nvalidations) <
isc_counter_getlimit(val->nvalidations))
{
return false;
}
@ -1270,14 +1273,14 @@ consume_validation(dns_validator_t *val) {
if (val->nvalidations == NULL) {
return;
}
INSIST((*val->nvalidations) > 0);
(*val->nvalidations)--;
(void)isc_counter_increment(val->nvalidations);
}
static bool
over_max_fails(dns_validator_t *val) {
if (val->nfails == NULL || (*val->nfails) > 0) {
if (val->nfails == NULL ||
isc_counter_used(val->nfails) < isc_counter_getlimit(val->nfails))
{
return false;
}
@ -1291,9 +1294,7 @@ consume_validation_fail(dns_validator_t *val) {
if (val->nfails == NULL) {
return;
}
INSIST((*val->nfails) > 0);
(*val->nfails)--;
(void)isc_counter_increment(val->nfails);
}
/*%
@ -3413,7 +3414,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset,
dns_message_t *message, unsigned int options,
isc_loop_t *loop, isc_job_cb cb, void *arg,
uint32_t *nvalidations, uint32_t *nfails,
isc_counter_t *nvalidations, isc_counter_t *nfails,
isc_counter_t *qc, isc_counter_t *gqc,
dns_edectx_t *edectx, dns_validator_t **validatorp) {
isc_result_t result = ISC_R_FAILURE;
@ -3445,8 +3446,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
.cb = cb,
.arg = arg,
.rdata = DNS_RDATA_INIT,
.nvalidations = nvalidations,
.nfails = nfails,
.edectx = edectx,
};
@ -3456,6 +3455,14 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
dns_message_attach(message, &val->message);
}
if (nfails != NULL) {
isc_counter_attach(nfails, &val->nfails);
}
if (nvalidations != NULL) {
isc_counter_attach(nvalidations, &val->nvalidations);
}
if (qc != NULL) {
isc_counter_attach(qc, &val->qc);
}
@ -3549,6 +3556,12 @@ destroy_validator(dns_validator_t *val) {
if (val->message != NULL) {
dns_message_detach(&val->message);
}
if (val->nfails != NULL) {
isc_counter_detach(&val->nfails);
}
if (val->nvalidations != NULL) {
isc_counter_detach(&val->nvalidations);
}
if (val->qc != NULL) {
isc_counter_detach(&val->qc);
}