CVE-2018-16852 dcerpc dnsserver: Ensure properties are handled correctly

Fixes for
Bug 13669 - (CVE-2018-16852) NULL
            pointer de-reference in Samba AD DC DNS management

The presence of the ZONE_MASTER_SERVERS property or the
ZONE_SCAVENGING_SERVERS property in a zone record causes the server to
follow a null pointer and terminate.

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/selftest/knownfail.d/bug13669 b/selftest/knownfail.d/bug13669
deleted file mode 100644 (file)
index 74c8c13..0000000
--- a/selftest/knownfail.d/bug13669
+++ /dev/null
@@ -1,4 +0,0 @@
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers_empty
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers_empty
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers

diff --git a/source4/rpc_server/dnsserver/dnsutils.c b/source4/rpc_server/dnsserver/dnsutils.c
index b3d8949f8abd079213271885348b75eeb1e43603..982b13bc2acab55d44771672126724271c6bcd04 100644 (file)
--- a/source4/rpc_server/dnsserver/dnsutils.c
+++ b/source4/rpc_server/dnsserver/dnsutils.c
@@ -209,6 +209,46 @@ struct dnsserver_serverinfo *dnsserver_init_serverinfo(TALLOC_CTX *mem_ctx,
 }
 
 
+/*
+ * 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)
 {
@@ -309,20 +349,28 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
                                prop->aging_enabled;
                        break;
                case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
-                       zoneinfo->aipScavengeServers->AddrCount =
-                               prop->servers.addrCount;
-                       zoneinfo->aipScavengeServers->AddrArray =
-                               prop->servers.addr;
+                       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->AddrCount =
-                               prop->master_servers.addrCount;
-                       zoneinfo->aipLocalMasters->AddrArray =
-                               prop->master_servers.addr;
+                       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: