From 42a070c507eb57e28b47952f8d3b8fd1ca69668e Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Thu, 18 Jun 2020 11:49:08 +0300 Subject: [PATCH 1/2] lib/ldb: add unit test for ldb_ldap internal code BUG: https://bugzilla.samba.org/show_bug.cgi?id=14413 Signed-off-by: Alexander Bokovoy Reviewed-by: Andreas Schneider (cherry picked from commit 36bd6edd8a603f3aa34aff81c43ef26efd3ad4cf) --- lib/ldb/tests/lldb_ldap.c | 105 ++++++++++++++++++++++++++++++++++++++ lib/ldb/wscript | 14 +++++ 2 files changed, 119 insertions(+) create mode 100644 lib/ldb/tests/lldb_ldap.c diff --git a/lib/ldb/tests/lldb_ldap.c b/lib/ldb/tests/lldb_ldap.c new file mode 100644 index 00000000000..eea9f22f6b9 --- /dev/null +++ b/lib/ldb/tests/lldb_ldap.c @@ -0,0 +1,105 @@ +/* + * 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 +#include +#include + +#include +#include +#include +#include + +int ldb_ldap_init(const char *version); + +#include "ldb_ldap/ldb_ldap.c" + +struct test_ctx { + struct tevent_context *ev; + struct ldb_context *ldb; + struct ldb_message *msg; +}; + +static int lldb_msg_setup(void **state) +{ + struct test_ctx *test_ctx; + + test_ctx = talloc_zero(NULL, struct test_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); + + test_ctx->msg = ldb_msg_new(test_ctx); + assert_non_null(test_ctx->msg); + + *state = test_ctx; + return 0; +} + +static int lldb_msg_teardown(void **state) +{ + struct test_ctx *test_ctx = talloc_get_type_abort(*state, + struct test_ctx); + + talloc_free(test_ctx); + return 0; +} + +static void test_lldb_add_msg_attr(void **state) +{ + struct test_ctx *test_ctx = talloc_get_type_abort(*state, + struct test_ctx); + struct ldb_message *msg = test_ctx->msg; + int ret; + unsigned int num_elements = 0; + struct berval **v = NULL; + + v = talloc_zero_array(test_ctx, struct berval *, 2); + assert_non_null(v); + + v[0] = talloc_zero(v, struct berval); + assert_non_null(v[0]); + + v[0]->bv_val = talloc_strdup(msg, "dc=example,dc=test"); + assert_non_null(v[0]->bv_val); + + v[0]->bv_len = strlen(v[0]->bv_val); + + num_elements = msg->num_elements; + + ret = lldb_add_msg_attr(test_ctx->ldb, msg, "defaultNamingContext", v); + assert_int_equal(ret, LDB_SUCCESS); + assert_int_equal(msg->num_elements, num_elements + 1); +} + + +int main(int argc, const char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown(test_lldb_add_msg_attr, + lldb_msg_setup, + lldb_msg_teardown), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/lib/ldb/wscript b/lib/ldb/wscript index 86b83c1b5cc..526fe497c13 100644 --- a/lib/ldb/wscript +++ b/lib/ldb/wscript @@ -527,6 +527,15 @@ def build(bld): deps='cmocka ldb ldb_tdb_err_map', install=False) + # If both libldap and liblber are available, test ldb_ldap + # code for a regression of bz#14413 -- even if we don't build + # it ourselves and simply using the system version + if bld.env.LIB_LDAP and bld.env.LIB_LBER: + bld.SAMBA_BINARY('lldb_ldap_test', + source='tests/lldb_ldap.c', + deps='cmocka talloc lber ldap ldb', + install=False) + if bld.CONFIG_SET('HAVE_LMDB'): bld.SAMBA_BINARY('ldb_mdb_mod_op_test', source='tests/ldb_mod_op_test.c', @@ -628,6 +637,11 @@ def test(ctx): # 'ldb_key_value_sub_txn_tdb_test' 'ldb_parse_test'] + # if LIB_LDAP and LIB_LBER defined, then we can test ldb_ldap backend + # behavior regression for bz#14413 + if env.LIB_LDAP and env.LIB_LBER: + test_exes += ["lldb_ldap_test"] + if env.HAVE_LMDB: test_exes += ['ldb_mdb_mod_op_test', 'ldb_lmdb_test', -- 2.25.4 From ff1f04705a853f4522289c54c9711a4d615106ce Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Thu, 18 Jun 2020 10:45:41 +0300 Subject: [PATCH 2/2] ldb_ldap: fix off-by-one increment in lldb_add_msg_attr Fix regression introduced by commit ce2bf5c72b6423fff680b3d6a9042103a6cdda55 lldb_add_msg_attr() calls ldb_msg_add_empty() which, in turn, calls calls _ldb_msg_add_el() which already increments msg->num_elements by one. As a result, msg->num_elements is bigger than the actual number of elements and any iteration over elements would step over elements array boundary. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14413 Signed-off-by: Alexander Bokovoy Reviewed-by: Andreas Schneider Autobuild-User(master): Andreas Schneider Autobuild-Date(master): Fri Jun 19 08:35:33 UTC 2020 on sn-devel-184 (cherry picked from commit 990a0fc4a0481aed817fad7575d8df453fbe7af9) --- lib/ldb/ldb_ldap/ldb_ldap.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/ldb/ldb_ldap/ldb_ldap.c b/lib/ldb/ldb_ldap/ldb_ldap.c index d7222997732..0531f8a62ae 100644 --- a/lib/ldb/ldb_ldap/ldb_ldap.c +++ b/lib/ldb/ldb_ldap/ldb_ldap.c @@ -176,8 +176,6 @@ static int lldb_add_msg_attr(struct ldb_context *ldb, el->num_values++; } - msg->num_elements++; - return 0; } -- 2.25.4