CVE-2018-16852 dcerpc dnsserver: refactor common properties handling

dnsserver_common.c and dnsutils.c both share similar code to process
zone properties.  This patch extracts the common code and moves it to
dnsserver_common.c.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13669

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index 1a032b4aa9f980920e066dd43942211e0dc03e0c..656d7ca6bff6a1fe82e3d7d85b16a6a1388c3d46 100644 (file)
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -742,6 +742,94 @@ bool dns_name_is_static(struct dnsp_DnssrvRpcRecord *records,
        return false;
 }
 
+/*
+ * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct.
+ * The new structure and it's data are allocated on the supplied talloc context
+ */
+static struct IP4_ARRAY *copy_ip4_array(TALLOC_CTX *ctx,
+                                       const char *name,
+                                       struct dnsp_ip4_array array)
+{
+
+       struct IP4_ARRAY *ip4_array = NULL;
+       unsigned int i;
+
+       ip4_array = talloc_zero(ctx, struct IP4_ARRAY);
+       if (ip4_array == NULL) {
+               DBG_ERR("Out of memory copying property [%s]\n", name);
+               return NULL;
+       }
+
+       ip4_array->AddrCount = array.addrCount;
+       if (ip4_array->AddrCount == 0) {
+               return ip4_array;
+       }
+
+       ip4_array->AddrArray =
+           talloc_array(ip4_array, uint32_t, ip4_array->AddrCount);
+       if (ip4_array->AddrArray == NULL) {
+               TALLOC_FREE(ip4_array);
+               DBG_ERR("Out of memory copying property [%s] values\n", name);
+               return NULL;
+       }
+
+       for (i = 0; i < ip4_array->AddrCount; i++) {
+               ip4_array->AddrArray[i] = array.addr[i];
+       }
+
+       return ip4_array;
+}
+
+bool dns_zoneinfo_load_zone_property(struct dnsserver_zoneinfo *zoneinfo,
+                                    struct dnsp_DnsProperty *prop)
+{
+       switch (prop->id) {
+       case DSPROPERTY_ZONE_TYPE:
+               zoneinfo->dwZoneType = prop->data.zone_type;
+               break;
+       case DSPROPERTY_ZONE_ALLOW_UPDATE:
+               zoneinfo->fAllowUpdate = prop->data.allow_update_flag;
+               break;
+       case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
+               zoneinfo->dwNoRefreshInterval = prop->data.norefresh_hours;
+               break;
+       case DSPROPERTY_ZONE_REFRESH_INTERVAL:
+               zoneinfo->dwRefreshInterval = prop->data.refresh_hours;
+               break;
+       case DSPROPERTY_ZONE_AGING_STATE:
+               zoneinfo->fAging = prop->data.aging_enabled;
+               break;
+       case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
+               zoneinfo->aipScavengeServers = copy_ip4_array(
+                   zoneinfo, "ZONE_SCAVENGING_SERVERS", prop->data.servers);
+               if (zoneinfo->aipScavengeServers == NULL) {
+                       return false;
+               }
+               break;
+       case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
+               zoneinfo->dwAvailForScavengeTime =
+                   prop->data.next_scavenging_cycle_hours;
+               break;
+       case DSPROPERTY_ZONE_MASTER_SERVERS:
+               zoneinfo->aipLocalMasters = copy_ip4_array(
+                   zoneinfo, "ZONE_MASTER_SERVERS", prop->data.master_servers);
+               if (zoneinfo->aipLocalMasters == NULL) {
+                       return false;
+               }
+               break;
+       case DSPROPERTY_ZONE_EMPTY:
+       case DSPROPERTY_ZONE_SECURE_TIME:
+       case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
+       case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
+       case DSPROPERTY_ZONE_DCPROMO_CONVERT:
+       case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
+       case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
+       case DSPROPERTY_ZONE_NS_SERVERS_DA:
+       case DSPROPERTY_ZONE_NODE_DBFLAGS:
+               break;
+       }
+       return true;
+}
 WERROR dns_get_zone_properties(struct ldb_context *samdb,
                               TALLOC_CTX *mem_ctx,
                               struct ldb_dn *zone_dn,
@@ -774,6 +862,7 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb,
        }
 
        for (i = 0; i < element->num_values; i++) {
+               bool valid_property;
                prop = talloc_zero(mem_ctx, struct dnsp_DnsProperty);
                if (prop == NULL) {
                        return WERR_NOT_ENOUGH_MEMORY;
@@ -787,42 +876,10 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb,
                        return DNS_ERR(SERVER_FAILURE);
                }
 
-               switch (prop->id) {
-               case DSPROPERTY_ZONE_AGING_STATE:
-                       zoneinfo->fAging = prop->data.aging_enabled;
-                       break;
-               case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
-                       zoneinfo->dwNoRefreshInterval =
-                           prop->data.norefresh_hours;
-                       break;
-               case DSPROPERTY_ZONE_REFRESH_INTERVAL:
-                       zoneinfo->dwRefreshInterval = prop->data.refresh_hours;
-                       break;
-               case DSPROPERTY_ZONE_ALLOW_UPDATE:
-                       zoneinfo->fAllowUpdate = prop->data.allow_update_flag;
-                       break;
-               case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
-                       zoneinfo->dwAvailForScavengeTime =
-                           prop->data.next_scavenging_cycle_hours;
-                       break;
-               case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
-                       zoneinfo->aipScavengeServers->AddrCount =
-                           prop->data.servers.addrCount;
-                       zoneinfo->aipScavengeServers->AddrArray =
-                           prop->data.servers.addr;
-                       break;
-               case DSPROPERTY_ZONE_EMPTY:
-               case DSPROPERTY_ZONE_TYPE:
-               case DSPROPERTY_ZONE_SECURE_TIME:
-               case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
-               case DSPROPERTY_ZONE_MASTER_SERVERS:
-               case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
-               case DSPROPERTY_ZONE_DCPROMO_CONVERT:
-               case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
-               case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
-               case DSPROPERTY_ZONE_NS_SERVERS_DA:
-               case DSPROPERTY_ZONE_NODE_DBFLAGS:
-                       break;
+               valid_property =
+                   dns_zoneinfo_load_zone_property(zoneinfo, prop);
+               if (!valid_property) {
+                       return DNS_ERR(SERVER_FAILURE);
                }
        }
 

diff --git a/source4/dns_server/dnsserver_common.h b/source4/dns_server/dnsserver_common.h
index 380f61b8dbc53f7454b9eb4eeedb63993c216c55..60ecde4fa917d085973377b10dafb1c5b0d38038 100644 (file)
--- a/source4/dns_server/dnsserver_common.h
+++ b/source4/dns_server/dnsserver_common.h
@@ -87,4 +87,7 @@ NTSTATUS dns_common_zones(struct ldb_context *samdb,
                          TALLOC_CTX *mem_ctx,
                          struct ldb_dn *base_dn,
                          struct dns_server_zone **zones_ret);
+
+bool dns_zoneinfo_load_zone_property(struct dnsserver_zoneinfo *zoneinfo,
+                                    struct dnsp_DnsProperty *prop);
 #endif /* __DNSSERVER_COMMON_H__ */

diff --git a/source4/rpc_server/dnsserver/dnsutils.c b/source4/rpc_server/dnsserver/dnsutils.c
index 982b13bc2acab55d44771672126724271c6bcd04..ea1eca596a13f3f5f68edbbf63dbd997dc3906f4 100644 (file)
--- a/source4/rpc_server/dnsserver/dnsutils.c
+++ b/source4/rpc_server/dnsserver/dnsutils.c
@@ -25,6 +25,7 @@
 #include "dsdb/samdb/samdb.h"
 #include "lib/socket/netif.h"
 #include "lib/util/util_net.h"
+#include "dnsserver_common.h"
 
 static struct DNS_ADDR_ARRAY *fill_dns_addr_array(TALLOC_CTX *mem_ctx,
                                           struct loadparm_context *lp_ctx,
@@ -208,47 +209,6 @@ struct dnsserver_serverinfo *dnsserver_init_serverinfo(TALLOC_CTX *mem_ctx,
        return serverinfo;
 }
 
-
-/*
- * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct.
- * The new structure and it's data are allocated on the supplied talloc context
- */
-static struct IP4_ARRAY *copy_ip4_array(
-       TALLOC_CTX *ctx,
-       const char *name,
-       struct dnsp_ip4_array array) {
-
-       struct IP4_ARRAY *ip4_array = NULL;
-       unsigned int i;
-
-       ip4_array = talloc_zero(ctx, struct IP4_ARRAY);
-       if (ip4_array == NULL) {
-               DBG_ERR("Out of memory copying property [%s]\n",
-                       name);
-               return NULL;
-       }
-
-       ip4_array->AddrCount = array.addrCount;
-       if (ip4_array->AddrCount == 0) {
-               return ip4_array;
-       }
-
-       ip4_array->AddrArray = talloc_array(ip4_array, uint32_t,
-                                           ip4_array->AddrCount);
-       if (ip4_array->AddrArray == NULL) {
-               TALLOC_FREE(ip4_array);
-               DBG_ERR("Out of memory copying property [%s] values\n",
-                       name);
-               return NULL;
-       }
-
-       for (i = 0; i < ip4_array->AddrCount; i++) {
-               ip4_array->AddrArray[i] = array.addr[i];
-       }
-
-       return ip4_array;
-}
-
 struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
                                                struct dnsserver_serverinfo *serverinfo)
 {
@@ -257,8 +217,7 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
        const char *revzone = "in-addr.arpa";
        const char *revzone6 = "ip6.arpa";
        int len1, len2;
-       union dnsPropertyData *prop = NULL;
-       int i=0;
+       unsigned int i = 0;
 
        zoneinfo = talloc_zero(zone, struct dnsserver_zoneinfo);
        if (zoneinfo == NULL) {
@@ -326,62 +285,12 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
        zoneinfo->dwLastXfrResult = 0;
 
        for(i=0; i<zone->num_props; i++){
-               prop=&(zone->tmp_props[i].data);
-               switch (zone->tmp_props[i].id) {
-               case DSPROPERTY_ZONE_TYPE:
-                       zoneinfo->dwZoneType =
-                               prop->zone_type;
-                       break;
-               case DSPROPERTY_ZONE_ALLOW_UPDATE:
-                       zoneinfo->fAllowUpdate =
-                               prop->allow_update_flag;
-                       break;
-               case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
-                       zoneinfo->dwNoRefreshInterval =
-                               prop->norefresh_hours;
-                       break;
-               case DSPROPERTY_ZONE_REFRESH_INTERVAL:
-                       zoneinfo->dwRefreshInterval =
-                               prop->refresh_hours;
-                       break;
-               case DSPROPERTY_ZONE_AGING_STATE:
-                       zoneinfo->fAging =
-                               prop->aging_enabled;
-                       break;
-               case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
-                       zoneinfo->aipScavengeServers =
-                               copy_ip4_array(zoneinfo,
-                                              "ZONE_SCAVENGING_SERVERS",
-                                              prop->servers);
-                       if (zoneinfo->aipScavengeServers == NULL) {
-                               TALLOC_FREE(zoneinfo);
-                               return NULL;
-                       }
-                       break;
-               case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
-                       zoneinfo->dwAvailForScavengeTime =
-                               prop->next_scavenging_cycle_hours;
-                       break;
-               case DSPROPERTY_ZONE_MASTER_SERVERS:
-                       zoneinfo->aipLocalMasters =
-                               copy_ip4_array(zoneinfo,
-                                              "ZONE_MASTER_SERVERS",
-                                              prop->master_servers);
-                       if (zoneinfo->aipLocalMasters == NULL) {
-                               TALLOC_FREE(zoneinfo);
-                               return NULL;
-                       }
-                       break;
-               case DSPROPERTY_ZONE_EMPTY:
-               case DSPROPERTY_ZONE_SECURE_TIME:
-               case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
-               case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
-               case DSPROPERTY_ZONE_DCPROMO_CONVERT:
-               case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
-               case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
-               case DSPROPERTY_ZONE_NS_SERVERS_DA:
-               case DSPROPERTY_ZONE_NODE_DBFLAGS:
-                       break;
+               bool valid_property;
+               valid_property = dns_zoneinfo_load_zone_property(
+                   zoneinfo, &zone->tmp_props[i]);
+               if (!valid_property) {
+                       TALLOC_FREE(zoneinfo);
+                       return NULL;
                }
        }