From 44a54a29d89f8c7fcb9bbea2c87f7c0bd0393c45 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 20 Nov 2024 01:20:42 +1100 Subject: [PATCH] Keep a local copy of the update rules to prevent UAF Previously, the update policy rules check was moved earlier in the sequence, and the keep rule match pointers were kept to maintain the ability to verify maximum records by type. However, these pointers can become invalid if server reloading or reconfiguration occurs before update completion. To prevent this issue, extract the maximum records by type value immediately during processing and only keep the copy of the values instead of the full ssurule. --- lib/ns/update.c | 70 ++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/lib/ns/update.c b/lib/ns/update.c index fb3bfb67a5..1b73d5ba7c 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -229,8 +229,8 @@ struct update { ns_client_t *client; isc_result_t result; dns_message_t *answer; - const dns_ssurule_t **rules; - size_t ruleslen; + unsigned int *maxbytype; + size_t maxbytypelen; }; /*% @@ -1639,8 +1639,8 @@ send_update(ns_client_t *client, dns_zone_t *zone) { dns_rdataclass_t zoneclass; dns_rdatatype_t covers; dns_name_t *zonename = NULL; - const dns_ssurule_t **rules = NULL; - size_t rule = 0, ruleslen = 0; + unsigned int *maxbytype = NULL; + size_t update = 0, maxbytypelen = 0; dns_zoneopt_t options; dns_db_t *db = NULL; dns_dbversion_t *ver = NULL; @@ -1685,21 +1685,22 @@ send_update(ns_client_t *client, dns_zone_t *zone) { * are illegal or violate policy. */ if (ssutable != NULL) { - ruleslen = request->counts[DNS_SECTION_UPDATE]; - rules = isc_mem_cget(mctx, ruleslen, sizeof(*rules)); + maxbytypelen = request->counts[DNS_SECTION_UPDATE]; + maxbytype = isc_mem_cget(mctx, maxbytypelen, + sizeof(*maxbytype)); } - for (rule = 0, + for (update = 0, result = dns_message_firstname(request, DNS_SECTION_UPDATE); - result == ISC_R_SUCCESS; - rule++, result = dns_message_nextname(request, DNS_SECTION_UPDATE)) + result == ISC_R_SUCCESS; update++, + result = dns_message_nextname(request, DNS_SECTION_UPDATE)) { dns_name_t *name = NULL; dns_rdata_t rdata = DNS_RDATA_INIT; dns_ttl_t ttl; dns_rdataclass_t update_class; - INSIST(ssutable == NULL || rule < ruleslen); + INSIST(ssutable == NULL || update < maxbytypelen); get_current_rr(request, DNS_SECTION_UPDATE, zoneclass, &name, &rdata, &covers, &ttl, &update_class); @@ -1775,6 +1776,8 @@ send_update(ns_client_t *client, dns_zone_t *zone) { dns_rdata_ptr_t ptr; dns_rdata_in_srv_t srv; + maxbytype[update] = 0; + isc_netaddr_fromsockaddr(&netaddr, &client->peeraddr); if (client->message->tsigkey != NULL) { @@ -1830,22 +1833,24 @@ send_update(ns_client_t *client, dns_zone_t *zone) { !dns_ssutable_checkrules( ssutable, client->signer, name, &netaddr, TCPCLIENT(client), env, - rdata.type, target, tsigkey, - &rules[rule])) + rdata.type, target, tsigkey, NULL)) { FAILC(DNS_R_REFUSED, "rejected by secure update"); } } else if (rdata.type != dns_rdatatype_any) { + const dns_ssurule_t *ssurule = NULL; if (!dns_ssutable_checkrules( ssutable, client->signer, name, &netaddr, TCPCLIENT(client), env, rdata.type, target, tsigkey, - &rules[rule])) + &ssurule)) { FAILC(DNS_R_REFUSED, "rejected by secure update"); } + maxbytype[update] = dns_ssurule_max(ssurule, + rdata.type); } else { if (!ssu_checkall(db, ver, name, ssutable, client->signer, &netaddr, env, @@ -1877,14 +1882,14 @@ send_update(ns_client_t *client, dns_zone_t *zone) { *uev = (update_t){ .zone = zone, .client = client, - .rules = rules, - .ruleslen = ruleslen, + .maxbytype = maxbytype, + .maxbytypelen = maxbytypelen, .result = ISC_R_SUCCESS, }; isc_nmhandle_attach(client->handle, &client->updatehandle); isc_async_run(dns_zone_getloop(zone), update_action, uev); - rules = NULL; + maxbytype = NULL; failure: if (db != NULL) { @@ -1892,8 +1897,8 @@ failure: dns_db_detach(&db); } - if (rules != NULL) { - isc_mem_cput(mctx, rules, ruleslen, sizeof(*rules)); + if (maxbytype != NULL) { + isc_mem_cput(mctx, maxbytype, maxbytypelen, sizeof(*maxbytype)); } if (ssutable != NULL) { @@ -2724,8 +2729,8 @@ update_action(void *arg) { update_t *uev = (update_t *)arg; dns_zone_t *zone = uev->zone; ns_client_t *client = uev->client; - const dns_ssurule_t **rules = uev->rules; - size_t rule = 0, ruleslen = uev->ruleslen; + unsigned int *maxbytype = uev->maxbytype; + size_t update = 0, maxbytypelen = uev->maxbytypelen; isc_result_t result; dns_db_t *db = NULL; dns_dbversion_t *oldver = NULL; @@ -2888,11 +2893,11 @@ update_action(void *arg) { /* * Process the Update Section. */ - INSIST(ssutable == NULL || rules != NULL); - for (rule = 0, + INSIST(ssutable == NULL || maxbytype != NULL); + for (update = 0, result = dns_message_firstname(request, DNS_SECTION_UPDATE); - result == ISC_R_SUCCESS; - rule++, result = dns_message_nextname(request, DNS_SECTION_UPDATE)) + result == ISC_R_SUCCESS; update++, + result = dns_message_nextname(request, DNS_SECTION_UPDATE)) { dns_name_t *name = NULL; dns_rdata_t rdata = DNS_RDATA_INIT; @@ -2900,14 +2905,12 @@ update_action(void *arg) { dns_rdataclass_t update_class; bool flag; - INSIST(ssutable == NULL || rule < ruleslen); + INSIST(ssutable == NULL || update < maxbytypelen); get_current_rr(request, DNS_SECTION_UPDATE, zoneclass, &name, &rdata, &covers, &ttl, &update_class); if (update_class == zoneclass) { - unsigned int max = 0; - /* * RFC1123 doesn't allow MF and MD in master files. */ @@ -3046,20 +3049,17 @@ update_action(void *arg) { } } - if (rules != NULL && rules[rule] != NULL) { - max = dns_ssurule_max(rules[rule], rdata.type); - } - if (max != 0) { + if (maxbytype != NULL && maxbytype[update] != 0) { unsigned int count = 0; CHECK(foreach_rr(db, ver, name, rdata.type, covers, count_action, &count)); - if (count >= max) { + if (count >= maxbytype[update]) { update_log(client, zone, LOGLEVEL_PROTOCOL, "attempt to add more " "records than permitted by " "policy max=%u", - max); + maxbytype[update]); continue; } } @@ -3437,8 +3437,8 @@ common: dns_db_detach(&db); } - if (rules != NULL) { - isc_mem_cput(mctx, rules, ruleslen, sizeof(*rules)); + if (maxbytype != NULL) { + isc_mem_cput(mctx, maxbytype, maxbytypelen, sizeof(*maxbytype)); } if (ssutable != NULL) {