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.

diff --git a/lib/ns/update.c b/lib/ns/update.c
index fb3bfb67a572f103e561a10ab355d32e43b3d8d9..1b73d5ba7c1b4e084308c7349222f2e573c8130d 100644 (file)
--- 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) {