librpc dnsp test: Ensure length matches union selector

Ensure that a dnsp_DnsProperty is rejected if the length data does not not
correspond to the length indicated by the union id.  It was possible for
the union to be referencing memory past the end of the structure.

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: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

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

diff --git a/source4/torture/ndr/dnsp.c b/source4/torture/ndr/dnsp.c
index 07bebeceb01e3c06940272b63e7c376e300cbf7c..3d0cd942d111151f91cfc85c565926b157fd5cfd 100644 (file)
--- a/source4/torture/ndr/dnsp.c
+++ b/source4/torture/ndr/dnsp.c
@@ -337,6 +337,23 @@ static bool dnsp_dnsProperty_deleted_by_check(struct torture_context *tctx,
        return true;
 }
 
+/*
+ * Copy of dnsp_dnsProperty_deleted_by_b64 with wDataLength set to 0
+ * and no data in the data element.
+ * This is a reproducer for https://bugzilla.samba.org/show_bug.cgi?id=14206
+ * The dns_property_id was retained so once parsed this structure referenced
+ * memory past it's end.
+ *
+ * [0000] 00 00 00 00 01 EE C4 71   00 00 00 00 01 00 00 00   &......q ........
+ * [0010] 80 00 00 00 77 00 32 00   6B 00 33 00 2D 00 31 00   ....w.2. k.3.-.1.
+ * [0020] 39 00 31 00 2E 00 77 00   32 00 6B 00 33 00 2E 00   9.1...w. 2.k.3...
+ * [0030] 62 00 61 00 73 00 65 00   00 00 C4 71 EC F3         b.a.s.e. ...q..
+ */
+static const uint8_t dnsp_dnsProperty_deleted_by_zero_wDataLength[] = {
+       0x00, 0x00, 0x00, 0x00, 0x01, 0xEE, 0xC4, 0x71, 0x00, 0x00, 0x00,
+       0x00, 0x01, 0x00, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00,
+       0xC4, 0x71, 0xEC, 0xF3 };
+
 struct torture_suite *ndr_dnsp_suite(TALLOC_CTX *ctx)
 {
        struct torture_suite *suite = torture_suite_create(ctx, "dnsp");
@@ -362,5 +379,11 @@ struct torture_suite *ndr_dnsp_suite(TALLOC_CTX *ctx)
                dnsp_dnsProperty_deleted_by_b64,
                dnsp_dnsProperty_deleted_by_check);
 
+       torture_suite_add_ndr_pull_invalid_data_test(
+               suite,
+               dnsp_DnsProperty,
+               dnsp_dnsProperty_deleted_by_zero_wDataLength,
+               NDR_ERR_BUFSIZE);
+
        return suite;
 }

diff --git a/source4/torture/ndr/ndr.c b/source4/torture/ndr/ndr.c
index e65a39ddb7f90b3a074aa6be2c50cb906152f993..32efe90757dd2683511cd6380820aed262ad356a 100644 (file)
--- a/source4/torture/ndr/ndr.c
+++ b/source4/torture/ndr/ndr.c
@@ -35,6 +35,7 @@ struct ndr_pull_test_data {
        ndr_print_function_t print_function;
        int ndr_flags;
        int flags;
+       enum ndr_err_code ndr_err;
 };
 
 static enum ndr_err_code torture_ndr_push_struct_blob_flags(DATA_BLOB *blob, TALLOC_CTX *mem_ctx, uint32_t flags, uint32_t ndr_flags, const void *p, ndr_push_flags_fn_t fn)
@@ -301,6 +302,72 @@ _PUBLIC_ struct torture_test *_torture_suite_add_ndr_pull_inout_test(
        return test;
 }
 
+static bool wrap_ndr_pull_invalid_data_test(struct torture_context *tctx,
+                                           struct torture_tcase *tcase,
+                                           struct torture_test *test)
+{
+       const struct ndr_pull_test_data *data = (const struct ndr_pull_test_data *)test->data;
+       struct ndr_pull *ndr = ndr_pull_init_blob(&(data->data), tctx);
+       void *ds = talloc_zero_size(ndr, data->struct_size);
+       bool ret = true;
+
+       torture_assert(tctx, data, "out of memory");
+       torture_assert(tctx, ndr, "out of memory");
+       torture_assert(tctx, ds, "out of memory");
+
+       ndr->flags |= data->flags;
+
+       ndr->flags |= LIBNDR_FLAG_REF_ALLOC;
+
+       torture_assert_ndr_err_equal(
+               tctx,
+               data->pull_fn(ndr, data->ndr_flags, ds),
+               NDR_ERR_BUFSIZE,
+               "pulling invalid data");
+
+       talloc_free(ndr);
+       return ret;
+}
+
+_PUBLIC_ struct torture_test *_torture_suite_add_ndr_pull_invalid_data_test(
+       struct torture_suite *suite,
+       const char *name,
+       ndr_pull_flags_fn_t pull_fn,
+       DATA_BLOB db,
+       size_t struct_size,
+       int ndr_flags,
+       int flags,
+       enum ndr_err_code ndr_err)
+{
+       struct torture_test *test;
+       struct torture_tcase *tcase;
+       struct ndr_pull_test_data *data;
+
+       tcase = torture_suite_add_tcase(suite, name);
+
+       test = talloc(tcase, struct torture_test);
+
+       test->name = talloc_strdup(test, name);
+       test->description = NULL;
+       test->run = wrap_ndr_pull_invalid_data_test;
+
+       data = talloc_zero(test, struct ndr_pull_test_data);
+       data->data = db;
+       data->ndr_flags = ndr_flags;
+       data->flags = flags;
+       data->struct_size = struct_size;
+       data->pull_fn = pull_fn;
+       data->ndr_err = ndr_err;
+
+       test->data = data;
+       test->fn = NULL;
+       test->dangerous = false;
+
+       DLIST_ADD_END(tcase->tests, test);
+
+       return test;
+}
+
 static bool test_check_string_terminator(struct torture_context *tctx)
 {
        struct ndr_pull *ndr;

diff --git a/source4/torture/ndr/ndr.h b/source4/torture/ndr/ndr.h
index 58ab19354ab5482008e857c8ec95d051717a1442..0cc21825fb4777231bf0aa607b0d534cd87297df 100644 (file)
--- a/source4/torture/ndr/ndr.h
+++ b/source4/torture/ndr/ndr.h
@@ -48,6 +48,16 @@ _PUBLIC_ struct torture_test *_torture_suite_add_ndr_pull_inout_test(
                                        int flags,
                                        bool (*check_fn) (struct torture_context *ctx, void *data));
 
+_PUBLIC_ struct torture_test *_torture_suite_add_ndr_pull_invalid_data_test(
+       struct torture_suite *suite,
+       const char *name,
+       ndr_pull_flags_fn_t pull_fn,
+       DATA_BLOB db,
+       size_t struct_size,
+       int ndr_flags,
+       int flags,
+       enum ndr_err_code ndr_err);
+
 #define torture_suite_add_ndr_pull_test(suite,name,data,check_fn) \
                _torture_suite_add_ndr_pullpush_test(suite, #name, \
                         (ndr_pull_flags_fn_t)ndr_pull_ ## name, \
@@ -59,6 +69,16 @@ _PUBLIC_ struct torture_test *_torture_suite_add_ndr_pull_inout_test(
                         NDR_SCALARS|NDR_BUFFERS, 0, \
                         (bool (*) (struct torture_context *, void *)) check_fn);
 
+#define torture_suite_add_ndr_pull_invalid_data_test(suite,name,data,ndr_err) \
+               _torture_suite_add_ndr_pull_invalid_data_test( \
+                       suite, \
+                       #name, \
+                       (ndr_pull_flags_fn_t)ndr_pull_ ## name, \
+                       data_blob_const(data, sizeof(data)), \
+                       sizeof(struct name), \
+                       NDR_SCALARS|NDR_BUFFERS, 0, \
+                       ndr_err);
+
 #define torture_suite_add_ndr_pull_fn_test(suite,name,data,flags,check_fn) \
                _torture_suite_add_ndr_pullpush_test(suite, #name "_" #flags, \
                         (ndr_pull_flags_fn_t)ndr_pull_ ## name, \