librpc/nbt: Avoid reading invalid member of union

WACK packets use the ‘data’ member of the ‘nbt_rdata’ union, but they
claim to be a different type — NBT_QTYPE_NETBIOS — than would normally
be used with that union member. This means that if rr_type is equal to
NBT_QTYPE_NETBIOS, ndr_push_nbt_res_rec() has to guess which type the
structure really is by examining the data member. However, if the
structure is actually of a different type, that union member will not be
valid and accessing it will invoke undefined behaviour.

To fix this, eliminate all the guesswork and introduce a new type,
NBT_QTYPE_WACK, which can never appear on the wire, and which indicates
that although the ‘data’ union member should be used, the wire type is
actually NBT_QTYPE_NETBIOS.

This means that as far as NDR is concerned, the ‘netbios’ member of the
‘nbt_rdata’ union will consistently be used for all NBT_QTYPE_NETBIOS
structures; we shall no longer access the wrong member of the union.

Credit to OSS-Fuzz.

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=38480

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
Autobuild-Date(master): Fri Jul  7 01:14:06 UTC 2023 on atb-devel-224

diff --git a/libcli/nbt/nbtname.c b/libcli/nbt/nbtname.c
index ec2b395d6815cca6d204d5f926c98277b9c69fb5..c4f2524021f85f428b320119d952c1ec88f75d50 100644 (file)
--- a/libcli/nbt/nbtname.c
+++ b/libcli/nbt/nbtname.c
@@ -478,23 +478,9 @@ _PUBLIC_ void ndr_print_wrepl_nbt_name(struct ndr_print *ndr, const char *name,
        talloc_free(s);
 }
 
-_PUBLIC_ enum ndr_err_code ndr_push_nbt_res_rec(struct ndr_push *ndr, int ndr_flags, const struct nbt_res_rec *r)
+_PUBLIC_ enum ndr_err_code ndr_push_nbt_qtype(struct ndr_push *ndr, int ndr_flags, enum nbt_qtype r)
 {
-       {
-               uint32_t _flags_save_STRUCT = ndr->flags;
-               ndr_set_flags(&ndr->flags, LIBNDR_PRINT_ARRAY_HEX);
-               if (ndr_flags & NDR_SCALARS) {
-                       NDR_CHECK(ndr_push_align(ndr, 4));
-                       NDR_CHECK(ndr_push_nbt_name(ndr, NDR_SCALARS, &r->name));
-                       NDR_CHECK(ndr_push_nbt_qtype(ndr, NDR_SCALARS, r->rr_type));
-                       NDR_CHECK(ndr_push_nbt_qclass(ndr, NDR_SCALARS, r->rr_class));
-                       NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, r->ttl));
-                       NDR_CHECK(ndr_push_set_switch_value(ndr, &r->rdata, ((((r->rr_type) == NBT_QTYPE_NETBIOS) && ((r->rdata).data.length == 2))?0:r->rr_type)));
-                       NDR_CHECK(ndr_push_nbt_rdata(ndr, NDR_SCALARS, &r->rdata));
-               }
-               if (ndr_flags & NDR_BUFFERS) {
-               }
-               ndr->flags = _flags_save_STRUCT;
-       }
+       /* For WACK replies, we need to send NBT_QTYPE_NETBIOS on the wire. */
+       NDR_CHECK(ndr_push_enum_uint16(ndr, NDR_SCALARS, (r == NBT_QTYPE_WACK) ? NBT_QTYPE_NETBIOS : r));
        return NDR_ERR_SUCCESS;
 }

diff --git a/librpc/idl/nbt.idl b/librpc/idl/nbt.idl
index fd56c46bb5e49108b43689e318d941627c0c2811..11814e7970e6a344a0da1deed4b8f55706b6a011 100644 (file)
--- a/librpc/idl/nbt.idl
+++ b/librpc/idl/nbt.idl
@@ -79,12 +79,18 @@ interface nbt
                NBT_QCLASS_IP = 0x01
        } nbt_qclass;
 
-       typedef [public,enum16bit] enum {
+       typedef [public,enum16bit,nopush] enum {
                NBT_QTYPE_ADDRESS     = 0x0001,
                NBT_QTYPE_NAMESERVICE = 0x0002,
                NBT_QTYPE_NULL        = 0x000A,
                NBT_QTYPE_NETBIOS     = 0x0020,
-               NBT_QTYPE_STATUS      = 0x0021
+               NBT_QTYPE_STATUS      = 0x0021,
+               /*
+                * Indicates that this is a WACK packet. As long as the size of
+                * ‘int’ is larger than 16 bits, this value cannot appear on the
+                * wire. We’ll encode it instead as NBT_QTYPE_NETBIOS.
+                */
+               NBT_QTYPE_WACK        = -1
        } nbt_qtype;
 
        typedef struct {
@@ -168,13 +174,7 @@ interface nbt
                [default]                 nbt_rdata_data   data;
        } nbt_rdata;
 
-/*
- * this macro works around the problem
- * that we need to use nbt_rdata_data
- * together with NBT_QTYPE_NETBIOS
- * for WACK replies
- */
-       typedef [flag(LIBNDR_PRINT_ARRAY_HEX),nopush] struct {
+       typedef [flag(LIBNDR_PRINT_ARRAY_HEX)] struct {
                nbt_name   name;
                nbt_qtype  rr_type;
                nbt_qclass rr_class;

diff --git a/source4/nbt_server/packet.c b/source4/nbt_server/packet.c
index b9aea56d3d252b0bfd7caaaba9acbe0732553893..1305d6546c5f06323a794777e470086bee769fe2 100644 (file)
--- a/source4/nbt_server/packet.c
+++ b/source4/nbt_server/packet.c
@@ -368,7 +368,7 @@ void nbtd_wack_reply(struct nbt_name_socket *nbtsock,
        if (packet->answers == NULL) goto failed;
 
        packet->answers[0].name              = *name;
-       packet->answers[0].rr_type           = NBT_QTYPE_NETBIOS;
+       packet->answers[0].rr_type           = NBT_QTYPE_WACK;
        packet->answers[0].rr_class          = NBT_QCLASS_IP;
        packet->answers[0].ttl               = ttl;
        packet->answers[0].rdata.data.length = 2;