librpc/idl/dnsserver.idl: Ensure DnsProperty id matches what is pulled from the store...

There are two concerns here, assuming the attacker can place arbitary values
in a dnsProperty attribute over LDAP (eg is a DNS administrator).

This comes from the fact that id is used as the switch value at the C layer
but at the NDR layer the wDataLength value is considered first.

One concern is that a pull/push round-trip could include server memory:

 The previous switch_is() behaviour could store the server memory back
 into the attribute.

 However this pattern of pull/push only happens in ndrdump and fuzzing tools, as
 dnsserver_db_do_reset_dword() operates only on the uint32/bitmap union
 arms, and fully initialises those.

The other is that a pull of the attacker-supplied value could
cause the server to expose memory.

 This would be over the network via DNS or the RPC dnsserver protocols.
 However at all times the ndr_pull_struct_blob is passed zeroed memory.

The final concern (which fuzz_ndr_X found) is that in the ndr_size_dnsPropertyData()
the union descriminent is only id.

 This has no impact as only zeroed memory is used so there will be a
 zero value in all scalars, including data->d_ns_servers.AddrArray.

 Therefore the server will not crash processing the attacker-supplied blob

[MS-DNSP] 2.3.2.1 dnsProperty has no mention of this special behaviour.
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dnsp/445c7843-e4a1-4222-8c0f-630c230a4c80

This was known as CVE-2019-14908 before being triaged back to a normal bug.

Found by Douglas Bagnall using Hongfuzz and the new fuzz_ndr_X fuzzer.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14206
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@samba.org>

diff --git a/librpc/idl/dnsp.idl b/librpc/idl/dnsp.idl
index 1bbcaca6bfa657ae12eb854b292ae696422ad800..814d573cddf3469d73049413ad5929376f4b00e3 100644 (file)
--- a/librpc/idl/dnsp.idl
+++ b/librpc/idl/dnsp.idl
@@ -257,7 +257,7 @@ interface dnsp
                [value(0)] uint32   flag;
                [value(1)] uint32   version;
                dns_property_id     id;
-               [switch_is(wDataLength?id:DSPROPERTY_ZONE_EMPTY)]     dnsPropertyData data;
+               [switch_is(id)]     dnsPropertyData data;
                uint32              name;
        } dnsp_DnsProperty;
 }

diff --git a/selftest/knownfail.d/bug-14206 b/selftest/knownfail.d/bug-14206
deleted file mode 100644 (file)
index e2d9439..0000000
--- a/selftest/knownfail.d/bug-14206
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba4.local.ndr.dnsp_DnsProperty\(none\)
-^samba4.local.ndr.system.iconv.dnsp_DnsProperty\(none\)