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.

(cherry picked from commit 44a54a29d89f8c7fcb9bbea2c87f7c0bd0393c45)

diff --git a/lib/ns/update.c b/lib/ns/update.c
index 1ee37e3e90759c8bdcf765dd492a76376bd5bc09..248f6ed53bf9ae238d9b2c70e6d1031b41eb6fec 100644 (file)
--- a/lib/ns/update.c
+++ b/lib/ns/update.c
@@ -231,8 +231,8 @@ struct update_event {
        dns_zone_t *zone;
        isc_result_t result;
        dns_message_t *answer;
-       const dns_ssurule_t **rules;
-       size_t ruleslen;
+       unsigned int *maxbytype;
+       size_t maxbytypelen;
 };
 
 /*%
@@ -1655,8 +1655,8 @@ send_update_event(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_db_t *db = NULL;
        dns_dbversion_t *ver = NULL;
 
@@ -1698,22 +1698,23 @@ send_update_event(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_get(mctx, sizeof(*rules) * ruleslen);
-               memset(rules, 0, sizeof(*rules) * ruleslen);
+               maxbytypelen = request->counts[DNS_SECTION_UPDATE];
+               maxbytype = isc_mem_get(mctx,
+                                       sizeof(*maxbytype) * maxbytypelen);
+               memset(maxbytype, 0, sizeof(*maxbytype) * maxbytypelen);
        }
 
-       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);
@@ -1778,6 +1779,8 @@ send_update_event(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) {
@@ -1833,22 +1836,24 @@ send_update_event(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,
@@ -1882,9 +1887,9 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) {
                sizeof(*event));
        event->zone = zone;
        event->result = ISC_R_SUCCESS;
-       event->rules = rules;
-       event->ruleslen = ruleslen;
-       rules = NULL;
+       event->maxbytype = maxbytype;
+       event->maxbytypelen = maxbytypelen;
+       maxbytype = NULL;
 
        INSIST(client->nupdates == 0);
        client->nupdates++;
@@ -1900,8 +1905,8 @@ failure:
                dns_db_detach(&db);
        }
 
-       if (rules != NULL) {
-               isc_mem_put(mctx, rules, sizeof(*rules) * ruleslen);
+       if (maxbytype != NULL) {
+               isc_mem_put(mctx, maxbytype, sizeof(*maxbytype) * maxbytypelen);
        }
 
        if (ssutable != NULL) {
@@ -2880,8 +2885,8 @@ update_action(isc_task_t *task, isc_event_t *event) {
        update_event_t *uev = (update_event_t *)event;
        dns_zone_t *zone = uev->zone;
        ns_client_t *client = (ns_client_t *)event->ev_arg;
-       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;
@@ -3049,11 +3054,11 @@ update_action(isc_task_t *task, isc_event_t *event) {
        /*
         * 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;
@@ -3061,14 +3066,12 @@ update_action(isc_task_t *task, isc_event_t *event) {
                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.
                         */
@@ -3207,20 +3210,17 @@ update_action(isc_task_t *task, isc_event_t *event) {
                                }
                        }
 
-                       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;
                                }
                        }
@@ -3695,8 +3695,8 @@ common:
                dns_db_detach(&db);
        }
 
-       if (rules != NULL) {
-               isc_mem_put(mctx, rules, sizeof(*rules) * ruleslen);
+       if (maxbytype != NULL) {
+               isc_mem_put(mctx, maxbytype, sizeof(*maxbytype) * maxbytypelen);
        }
 
        if (ssutable != NULL) {