CVE-2018-16852 dcerpc dnsserver: Verification tests

Tests to verify
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
new file mode 100644 (file)
index 0000000..74c8c13
--- /dev/null
+++ b/selftest/knownfail.d/bug13669
@@ -0,0 +1,4 @@
+^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/tests/rpc_dns_server_dnsutils_test.c b/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c
new file mode 100644 (file)
index 0000000..8972113
--- /dev/null
+++ b/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c
@@ -0,0 +1,304 @@
+/*
+ * Unit tests for source4/rpc_server/dnsserver/dnsutils.c
+ *
+ *  Copyright (C) Catalyst.NET Ltd 2018
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * from cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ *
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+
+#include "../dnsserver/dnsutils.c"
+
+
+/*
+ * Test setting of an empty ZONE_MASTER_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_master_servers_empty(void **state)
+{
+       struct dnsserver_zone *zone = NULL;
+       struct dnsserver_serverinfo *serverinfo = NULL;
+       struct dnsserver_zoneinfo *zoneinfo = NULL;
+       struct dnsp_DnsProperty *property = NULL;
+
+       TALLOC_CTX *ctx = talloc_new(NULL);
+
+       /*
+        * Setup the zone data
+        */
+       zone = talloc_zero(ctx, struct dnsserver_zone);
+       assert_non_null(zone);
+       zone->name = "test";
+
+       /*
+        * Set up an empty ZONE_MASTER_SERVERS property
+        */
+       property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+       assert_non_null(property);
+       property->id = DSPROPERTY_ZONE_MASTER_SERVERS;
+       property->data.master_servers.addrCount = 0;
+       property->data.master_servers.addr = NULL;
+
+       zone->tmp_props = property;
+       zone->num_props = 1;
+
+
+       /*
+        * Setup the server info
+        */
+       serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+       assert_non_null(serverinfo);
+
+       /*
+        * call dnsserver_init_zoneinfo
+        */
+       zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+       /*
+        * Check results
+        */
+       assert_non_null(zoneinfo);
+       assert_non_null(zoneinfo->aipLocalMasters);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrCount, 0);
+       assert_null(zoneinfo->aipLocalMasters->AddrArray);
+
+       TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of a non empty ZONE_MASTER_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_master_servers(void **state)
+{
+       struct dnsserver_zone *zone = NULL;
+       struct dnsserver_serverinfo *serverinfo = NULL;
+       struct dnsserver_zoneinfo *zoneinfo = NULL;
+       struct dnsp_DnsProperty *property = NULL;
+
+       TALLOC_CTX *ctx = talloc_new(NULL);
+
+       /*
+        * Setup the zone data
+        */
+       zone = talloc_zero(ctx, struct dnsserver_zone);
+       assert_non_null(zone);
+       zone->name = "test";
+
+       /*
+        * Set up an empty ZONE_MASTER_SERVERS property
+        */
+       property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+       assert_non_null(property);
+       property->id = DSPROPERTY_ZONE_MASTER_SERVERS;
+       property->data.master_servers.addrCount = 4;
+       property->data.master_servers.addr =
+               talloc_zero_array(ctx, uint32_t, 4);
+       property->data.master_servers.addr[0] = 1000;
+       property->data.master_servers.addr[1] = 1001;
+       property->data.master_servers.addr[2] = 1002;
+       property->data.master_servers.addr[3] = 1003;
+
+       zone->tmp_props = property;
+       zone->num_props = 1;
+
+
+       /*
+        * Setup the server info
+        */
+       serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+       assert_non_null(serverinfo);
+
+       /*
+        * call dnsserver_init_zoneinfo
+        */
+       zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+       /*
+        * Check results
+        */
+       assert_non_null(zoneinfo);
+       assert_non_null(zoneinfo->aipLocalMasters);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrCount, 4);
+       assert_non_null(zoneinfo->aipLocalMasters->AddrArray);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[0], 1000);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[1], 1001);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[2], 1002);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[3], 1003);
+
+       /*
+        * Ensure that we're working with a copy of the property data
+        * and not a reference.
+        * The pointers should be different, and we should be able to change
+        * the values in the property without affecting the zoneinfo data
+        */
+       assert_true(zoneinfo->aipLocalMasters->AddrArray !=
+                   property->data.master_servers.addr);
+       property->data.master_servers.addr[0] = 0;
+       property->data.master_servers.addr[1] = 1;
+       property->data.master_servers.addr[2] = 2;
+       property->data.master_servers.addr[3] = 3;
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[0], 1000);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[1], 1001);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[2], 1002);
+       assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[3], 1003);
+
+       TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of an empty ZONE_SCAVENGING_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_scavenging_servers_empty(void **state)
+{
+       struct dnsserver_zone *zone = NULL;
+       struct dnsserver_serverinfo *serverinfo = NULL;
+       struct dnsserver_zoneinfo *zoneinfo = NULL;
+       struct dnsp_DnsProperty *property = NULL;
+
+       TALLOC_CTX *ctx = talloc_new(NULL);
+
+       /*
+        * Setup the zone data
+        */
+       zone = talloc_zero(ctx, struct dnsserver_zone);
+       assert_non_null(zone);
+       zone->name = "test";
+
+       property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+       assert_non_null(property);
+       property->id = DSPROPERTY_ZONE_SCAVENGING_SERVERS;
+       property->data.servers.addrCount = 0;
+       property->data.servers.addr = NULL;
+
+       zone->tmp_props = property;
+       zone->num_props = 1;
+
+
+       serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+       assert_non_null(serverinfo);
+
+       zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+       assert_non_null(zoneinfo);
+       assert_non_null(zoneinfo->aipScavengeServers);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrCount, 0);
+       assert_null(zoneinfo->aipScavengeServers->AddrArray);
+
+       TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of a non empty ZONE_SCAVENGING_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_scavenging_servers(void **state)
+{
+       struct dnsserver_zone *zone = NULL;
+       struct dnsserver_serverinfo *serverinfo = NULL;
+       struct dnsserver_zoneinfo *zoneinfo = NULL;
+       struct dnsp_DnsProperty *property = NULL;
+
+       TALLOC_CTX *ctx = talloc_new(NULL);
+
+       /*
+        * Setup the zone data
+        */
+       zone = talloc_zero(ctx, struct dnsserver_zone);
+       assert_non_null(zone);
+       zone->name = "test";
+
+       property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+       assert_non_null(property);
+       property->id = DSPROPERTY_ZONE_SCAVENGING_SERVERS;
+       property->data.servers.addrCount = 4;
+       property->data.servers.addr = talloc_zero_array(ctx, uint32_t, 4);
+       property->data.servers.addr[0] = 1000;
+       property->data.servers.addr[1] = 1001;
+       property->data.servers.addr[2] = 1002;
+       property->data.servers.addr[3] = 1003;
+
+       zone->tmp_props = property;
+       zone->num_props = 1;
+
+
+       serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+       assert_non_null(serverinfo);
+
+       zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+       assert_non_null(zoneinfo);
+       assert_non_null(zoneinfo->aipScavengeServers);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrCount, 4);
+       assert_non_null(zoneinfo->aipScavengeServers->AddrArray);
+       assert_non_null(zoneinfo->aipScavengeServers->AddrArray);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[0], 1000);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[1], 1001);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[2], 1002);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[3], 1003);
+
+       /*
+        * Ensure that we're working with a copy of the property data
+        * and not a reference.
+        * The pointers should be different, and we should be able to change
+        * the values in the property without affecting the zoneinfo data
+        */
+       assert_true(zoneinfo->aipScavengeServers->AddrArray !=
+                   property->data.servers.addr);
+       property->data.servers.addr[0] = 0;
+       property->data.servers.addr[1] = 1;
+       property->data.servers.addr[2] = 2;
+       property->data.servers.addr[3] = 3;
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[0], 1000);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[1], 1001);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[2], 1002);
+       assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[3], 1003);
+
+
+       TALLOC_FREE(ctx);
+}
+int main(int argc, const char **argv)
+{
+       const struct CMUnitTest tests[] = {
+               cmocka_unit_test(
+                   test_dnsserver_init_zoneinfo_master_servers_empty),
+               cmocka_unit_test(
+                   test_dnsserver_init_zoneinfo_master_servers),
+               cmocka_unit_test(
+                   test_dnsserver_init_zoneinfo_scavenging_servers_empty),
+               cmocka_unit_test(
+                   test_dnsserver_init_zoneinfo_scavenging_servers),
+       };
+
+       cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+       return cmocka_run_group_tests(tests, NULL, NULL);
+}

diff --git a/source4/rpc_server/wscript_build b/source4/rpc_server/wscript_build
index 8e05eb8a0c3bbe4266debd053e5664b6ba36e208..14b68c7ce0fa64bc8bd5a739e691904b74eb7df3 100644 (file)
--- a/source4/rpc_server/wscript_build
+++ b/source4/rpc_server/wscript_build
@@ -3,7 +3,7 @@
 bld.SAMBA_SUBSYSTEM('DCERPC_SHARE',
        source='common/share_info.c',
        autoproto='common/share.h',
-       deps='ldb',
+       deps='ldb share',
        enabled=bld.CONFIG_SET('WITH_NTVFS_FILESERVER'),
        )
 
@@ -163,7 +163,7 @@ bld.SAMBA_MODULE('dcerpc_dnsserver',
     source='dnsserver/dcerpc_dnsserver.c dnsserver/dnsutils.c dnsserver/dnsdata.c dnsserver/dnsdb.c',
     subsystem='dcerpc_server',
     init_function='dcerpc_server_dnsserver_init',
-    deps='DCERPC_COMMON dnsserver_common'
+    deps='DCERPC_COMMON dnsserver_common netif'
     )
 
 
@@ -176,3 +176,16 @@ bld.SAMBA_MODULE('service_dcerpc',
        deps='dcerpc_server'
        )
 
+if bld.CONFIG_GET('ENABLE_SELFTEST'):
+    bld.SAMBA_BINARY(
+        'test_rpc_dns_server_dnsutils',
+        source='tests/rpc_dns_server_dnsutils_test.c',
+        deps='''
+            dnsserver_common
+            DCERPC_COMMON
+            cmocka
+            talloc
+        ''',
+        install=False,
+        enabled=bld.AD_DC_BUILD_IS_ENABLED()
+    )

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index dc5807247ab103361c1bf7deaf0d0e1ac7ba117f..972f65307d867ba1a6406cb5d6523027837ef391 100755 (executable)
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1237,6 +1237,8 @@ plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit", "none",
               [os.path.join(bindir(), "test_group_audit")])
 plantestsuite("samba4.dsdb.samdb.ldb_modules.group_audit.errors", "none",
               [os.path.join(bindir(), "test_group_audit_errors")])
+plantestsuite("samba4.dcerpc.dnsserver.dnsutils", "none",
+              [os.path.join(bindir(), "test_rpc_dns_server_dnsutils")])
 
 # process restart and limit tests, these break the environment so need to run
 # in their own specific environment