From 53036b35f6b2a737a7c290595053a7dd9626cf5b Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 9 Dec 2019 09:19:47 +1300 Subject: [PATCH] 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 --- source4/torture/ndr/dnsp.c | 23 +++++++++++++ source4/torture/ndr/ndr.c | 67 ++++++++++++++++++++++++++++++++++++++ source4/torture/ndr/ndr.h | 20 ++++++++++++ 3 files changed, 110 insertions(+) diff --git a/source4/torture/ndr/dnsp.c b/source4/torture/ndr/dnsp.c index 07bebeceb01..3d0cd942d11 100644 --- 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 e65a39ddb7f..32efe90757d 100644 --- 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 58ab19354ab..0cc21825fb4 100644 --- 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, \ -- 2.17.1