From 45b75db50f5c1a7c8c38af59a62fccee5401c845 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Tue, 19 Feb 2019 10:24:38 +1300 Subject: [PATCH] CVE-2019-3824 ldb: Add tests for ldb_wildcard_match Add cmocka tests for ldb_wildcard_match. Running test_wildcard_match under valgrind reproduces CVE-2019-3824 out of bounds read in wildcard compare (bug 13773) valgrind --suppressions=lib/ldb/tests/ldb_match_test.valgrind\ bin/ldb_match_test BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773 Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett --- lib/ldb/tests/ldb_match_test.c | 191 ++++++++++++++++++++++++++ lib/ldb/tests/ldb_match_test.valgrind | 16 +++ lib/ldb/wscript | 8 +- 3 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 lib/ldb/tests/ldb_match_test.c create mode 100644 lib/ldb/tests/ldb_match_test.valgrind diff --git a/lib/ldb/tests/ldb_match_test.c b/lib/ldb/tests/ldb_match_test.c new file mode 100644 index 00000000000..e09f50c86ba --- /dev/null +++ b/lib/ldb/tests/ldb_match_test.c @@ -0,0 +1,191 @@ +/* + * Tests exercising the ldb match operations. + * + * + * Copyright (C) Catalyst.NET Ltd 2017 + * + * 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 . + * + */ + +/* + * from cmocka.c: + * These headers or their equivalents should be included prior to + * including + * this header file. + * + * #include + * #include + * #include + * + * This allows test applications to use custom definitions of C standard + * library functions and types. + */ +#include +#include +#include +#include +#include + +#include "../common/ldb_match.c" + +#include "../include/ldb.h" + +struct ldbtest_ctx { + struct tevent_context *ev; + struct ldb_context *ldb; +}; + +static int ldb_test_canonicalise( + struct ldb_context *ldb, + void *mem_ctx, + const struct ldb_val *in, + struct ldb_val *out) +{ + out->length = in->length; + out->data = in->data; + return 0; +} + +static int setup(void **state) +{ + struct ldbtest_ctx *test_ctx; + struct ldb_schema_syntax *syntax = NULL; + int ret; + + test_ctx = talloc_zero(NULL, struct ldbtest_ctx); + assert_non_null(test_ctx); + + test_ctx->ev = tevent_context_init(test_ctx); + assert_non_null(test_ctx->ev); + + test_ctx->ldb = ldb_init(test_ctx, test_ctx->ev); + assert_non_null(test_ctx->ldb); + + syntax = talloc_zero(test_ctx, struct ldb_schema_syntax); + assert_non_null(syntax); + syntax->canonicalise_fn = ldb_test_canonicalise; + + ret = ldb_schema_attribute_add_with_syntax( + test_ctx->ldb, "a", LDB_ATTR_FLAG_FIXED, syntax); + assert_int_equal(LDB_SUCCESS, ret); + + *state = test_ctx; + return 0; +} + +static int teardown(void **state) +{ + talloc_free(*state); + return 0; +} + + +/* + * The wild card pattern "attribute=*" is parsed as an LDB_OP_PRESENT operation + * rather than a LDB_OP_???? + * + * This test serves to document that behaviour, and to confirm that + * ldb_wildcard_compare handles this case appropriately. + */ +static void test_wildcard_match_star(void **state) +{ + struct ldbtest_ctx *ctx = *state; + bool matched = false; + int ret; + + uint8_t value[] = "The value.......end"; + struct ldb_val val = { + .data = value, + .length = (sizeof(value)) + }; + struct ldb_parse_tree *tree = ldb_parse_tree(ctx, "a=*"); + assert_non_null(tree); + + ret = ldb_wildcard_compare(ctx->ldb, tree, val, &matched); + assert_false(matched); + assert_int_equal(LDB_ERR_INAPPROPRIATE_MATCHING, ret); +} + +/* + * Test basic wild card matching + * + */ +static void test_wildcard_match(void **state) +{ + struct ldbtest_ctx *ctx = *state; + bool matched = false; + + uint8_t value[] = "The value.......end"; + struct ldb_val val = { + .data = value, + .length = (sizeof(value)) + }; + struct ldb_parse_tree *tree = ldb_parse_tree(ctx, "objectClass=*end"); + assert_non_null(tree); + + ldb_wildcard_compare(ctx->ldb, tree, val, &matched); + assert_true(matched); +} + + +/* + * ldb_handler_copy and ldb_val_dup over allocate by one and add a trailing '\0' + * to the data, to make them safe to use the C string functions on. + * + * However testing for the trailing '\0' is not the correct way to test for + * the end of a value, the length should be checked instead. + */ +static void test_wildcard_match_end_condition(void **state) +{ + struct ldbtest_ctx *ctx = *state; + bool matched = false; + + uint8_t value[] = "hellomynameisbobx"; + struct ldb_val val = { + .data = talloc_memdup(NULL, value, sizeof(value)), + .length = (sizeof(value) - 2) + }; + struct ldb_parse_tree *tree = ldb_parse_tree(ctx, "a=*hello*mynameis*bob"); + assert_non_null(tree); + + ldb_wildcard_compare(ctx->ldb, tree, val, &matched); + assert_true(matched); +} + +/* + * Note: to run under valgrind use: + * valgrind \ + * --suppressions=lib/ldb/tests/ldb_match_test.valgrind \ + * bin/ldb_match_test + */ +int main(int argc, const char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown( + test_wildcard_match_star, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_wildcard_match, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_wildcard_match_end_condition, + setup, + teardown), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/lib/ldb/tests/ldb_match_test.valgrind b/lib/ldb/tests/ldb_match_test.valgrind new file mode 100644 index 00000000000..660bd5a6b46 --- /dev/null +++ b/lib/ldb/tests/ldb_match_test.valgrind @@ -0,0 +1,16 @@ +{ + Memory allocated in set-up + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + ... + fun:setup +} +{ + Memory allocated by ldb_init + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + ... + fun:ldb_init +} diff --git a/lib/ldb/wscript b/lib/ldb/wscript index 6e224e7b4b7..5355585f69c 100644 --- a/lib/ldb/wscript +++ b/lib/ldb/wscript @@ -511,6 +511,11 @@ def build(bld): deps='cmocka ldb', install=False) + bld.SAMBA_BINARY('ldb_match_test', + source='tests/ldb_match_test.c', + deps='cmocka ldb', + install=False) + if bld.CONFIG_SET('HAVE_LMDB'): bld.SAMBA_BINARY('ldb_mdb_mod_op_test', source='tests/ldb_mod_op_test.c', @@ -578,7 +583,8 @@ def test(ctx): # we don't want to run ldb_lmdb_size_test (which proves we can # fit > 4G of data into the DB), it would fill up the disk on # many of our test instances - 'ldb_mdb_kv_ops_test'] + 'ldb_mdb_kv_ops_test', + 'ldb_match_test'] for test_exe in test_exes: cmd = os.path.join(Context.g_module.out, test_exe)