[SCM] Samba Shared Repository - branch v4-8-test updated

Stefan Metzmacher metze at samba.org
Tue Feb 26 11:59:01 UTC 2019


The branch, v4-8-test has been updated
       via  8be2836cd82 PVE-2019-3824 ldb: Release ldb 1.3.8
       via  a6b067e00b6 CVE-2019-3824 ldb: Add tests for ldb_wildcard_match
       via  2f6b4d11136 CVE-2019-3824 ldb: wildcard_match end of data check
       via  9b5a7c8abec CVE-2019-3824 ldb: wildcard_match check tree operation
       via  da12e534efe CVE-2019-3824 ldb: ldb_parse_tree use talloc_zero
       via  699e2aa1994 CVE-2019-3824 ldb: Improve code style and layout in wildcard processing
       via  28193ca851c CVE-2019-3824 ldb: Extra comments to clarify no pointer wrap in wildcard processing
       via  bd62896ddc2 CVE-2019-3824 ldb: Out of bound read in ldb_wildcard_compare
      from  080dae06412 waf: Check for libnscd

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-8-test


- Log -----------------------------------------------------------------
commit 8be2836cd825054ecffe112226400cdc42a2afc3
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Wed Feb 20 10:45:05 2019 +1300

    PVE-2019-3824 ldb: Release ldb 1.3.8
    
    * CVE-2019-3824 out of bounds read in wildcard compare (bug 13773)
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    
    Autobuild-User(v4-8-test): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(v4-8-test): Tue Feb 26 12:58:03 CET 2019 on sn-devel-144

commit a6b067e00b67cac6f3a36c8ef5edba6fd9b10def
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Tue Feb 19 10:24:38 2019 +1300

    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 <gary at catalyst.net.nz>

commit 2f6b4d11136f042f5c532199389877ed846c6f83
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Tue Feb 19 10:26:56 2019 +1300

    CVE-2019-3824 ldb: wildcard_match end of data check
    
    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.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>

commit 9b5a7c8abecbf605227cc974927c6d76f9bbbbb5
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Tue Feb 19 10:26:25 2019 +1300

    CVE-2019-3824 ldb: wildcard_match check tree operation
    
    Check the operation type of the passed parse tree, and return
    LDB_INAPPROPRIATE_MATCH if the operation is not LDB_OP_SUBSTRING.
    
    A query of "attribute=*" gets parsed as LDB_OP_PRESENT, checking the
    operation and failing ldb_wildcard_match should help prevent confusion
    writing tests.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>

commit da12e534efe2c80dc394295315a9a34ac72a2e9f
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Tue Feb 19 10:25:24 2019 +1300

    CVE-2019-3824 ldb: ldb_parse_tree use talloc_zero
    
    Initialise the created ldb_parse_tree with talloc_zero, this ensures
    that it is correctly initialised if inadvertently passed to a function
    expecting a different operation type.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>

commit 699e2aa19946d43b162355dcb299a1dd798c9cd7
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Feb 4 11:22:50 2019 +1300

    CVE-2019-3824 ldb: Improve code style and layout in wildcard processing
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 28193ca851ccba9652f59a2ba4213f536c9fa198
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Feb 4 11:22:34 2019 +1300

    CVE-2019-3824 ldb: Extra comments to clarify no pointer wrap in wildcard processing
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit bd62896ddc223270082dd67b068e944c696fed09
Author: Lukas Slebodnik <lslebodn at fedoraproject.org>
Date:   Fri Jan 18 16:37:24 2019 +0100

    CVE-2019-3824 ldb: Out of bound read in ldb_wildcard_compare
    
    There is valgrind error in few tests tests/test-generic.sh
     91 echo "Test wildcard match"
     92 $VALGRIND ldbadd $LDBDIR/tests/test-wildcard.ldif  || exit 1
     93 $VALGRIND ldbsearch '(cn=test*multi)'  || exit 1
     95 $VALGRIND ldbsearch '(cn=*test_multi)'  || exit 1
     97 $VALGRIND ldbsearch '(cn=test*multi*test*multi)'  || exit 1
    
    e.g.
      ==3098== Memcheck, a memory error detector
      ==3098== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
      ==3098== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
      ==3098== Command: ./bin/ldbsearch (cn=test*multi)
      ==3098==
      ==3098== Invalid read of size 1
      ==3098==    at 0x483CEE7: memchr (vg_replace_strmem.c:890)
      ==3098==    by 0x49A9073: memmem (in /usr/lib64/libc-2.28.9000.so)
      ==3098==    by 0x485DFE9: ldb_wildcard_compare (ldb_match.c:313)
      ==3098==    by 0x485DFE9: ldb_match_substring (ldb_match.c:360)
      ==3098==    by 0x485DFE9: ldb_match_message (ldb_match.c:572)
      ==3098==    by 0x558F8FA: search_func (ldb_kv_search.c:549)
      ==3098==    by 0x48C78CA: ??? (in /usr/lib64/libtdb.so.1.3.17)
      ==3098==    by 0x48C7A60: tdb_traverse_read (in /usr/lib64/libtdb.so.1.3.17)
      ==3098==    by 0x557B7C4: ltdb_traverse_fn (ldb_tdb.c:274)
      ==3098==    by 0x558FBFA: ldb_kv_search_full (ldb_kv_search.c:594)
      ==3098==    by 0x558FBFA: ldb_kv_search (ldb_kv_search.c:854)
      ==3098==    by 0x558E497: ldb_kv_callback (ldb_kv.c:1713)
      ==3098==    by 0x48FCD58: tevent_common_invoke_timer_handler (in /usr/lib64/libtevent.so.0.9.38)
      ==3098==    by 0x48FCEFD: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.38)
      ==3098==    by 0x48FE14A: ??? (in /usr/lib64/libtevent.so.0.9.38)
      ==3098==  Address 0x4b4ab81 is 0 bytes after a block of size 129 alloc'd
      ==3098==    at 0x483880B: malloc (vg_replace_malloc.c:309)
      ==3098==    by 0x491048B: talloc_strndup (in /usr/lib64/libtalloc.so.2.1.15)
      ==3098==    by 0x48593CA: ldb_casefold_default (ldb_utf8.c:59)
      ==3098==    by 0x485F68D: ldb_handler_fold (attrib_handlers.c:64)
      ==3098==    by 0x485DB88: ldb_wildcard_compare (ldb_match.c:257)
      ==3098==    by 0x485DB88: ldb_match_substring (ldb_match.c:360)
      ==3098==    by 0x485DB88: ldb_match_message (ldb_match.c:572)
      ==3098==    by 0x558F8FA: search_func (ldb_kv_search.c:549)
      ==3098==    by 0x48C78CA: ??? (in /usr/lib64/libtdb.so.1.3.17)
      ==3098==    by 0x48C7A60: tdb_traverse_read (in /usr/lib64/libtdb.so.1.3.17)
      ==3098==    by 0x557B7C4: ltdb_traverse_fn (ldb_tdb.c:274)
      ==3098==    by 0x558FBFA: ldb_kv_search_full (ldb_kv_search.c:594)
      ==3098==    by 0x558FBFA: ldb_kv_search (ldb_kv_search.c:854)
      ==3098==    by 0x558E497: ldb_kv_callback (ldb_kv.c:1713)
      ==3098==    by 0x48FCD58: tevent_common_invoke_timer_handler (in /usr/lib64/libtevent.so.0.9.38)
      ==3098==
      # record 1
      dn: cn=test_multi_test_multi_test_multi,o=University of Michigan,c=TEST
      cn: test_multi_test_multi_test_multi
      description: test multi wildcards matching
      objectclass: person
      sn: multi_test
      name: test_multi_test_multi_test_multi
      distinguishedName: cn=test_multi_test_multi_test_multi,o=University of Michiga
       n,c=TEST
    
      # returned 1 records
      # 1 entries
      # 0 referrals
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
    
    Signed-off-by: Lukas Slebodnik <lslebodn at fedoraproject.org>

-----------------------------------------------------------------------

Summary of changes:
 lib/ldb/ABI/{ldb-1.3.0.sigs => ldb-1.3.8.sigs}     |   0
 ...yldb-util-1.1.10.sigs => pyldb-util-1.3.8.sigs} |   0
 ...-util-1.1.10.sigs => pyldb-util.py3-1.3.8.sigs} |   0
 lib/ldb/common/ldb_match.c                         |  41 ++++-
 lib/ldb/common/ldb_parse.c                         |   2 +-
 lib/ldb/tests/ldb_match_test.c                     | 191 +++++++++++++++++++++
 lib/ldb/tests/ldb_match_test.valgrind              |  16 ++
 lib/ldb/wscript                                    |  10 +-
 8 files changed, 251 insertions(+), 9 deletions(-)
 copy lib/ldb/ABI/{ldb-1.3.0.sigs => ldb-1.3.8.sigs} (100%)
 copy lib/ldb/ABI/{pyldb-util-1.1.10.sigs => pyldb-util-1.3.8.sigs} (100%)
 copy lib/ldb/ABI/{pyldb-util-1.1.10.sigs => pyldb-util.py3-1.3.8.sigs} (100%)
 create mode 100644 lib/ldb/tests/ldb_match_test.c
 create mode 100644 lib/ldb/tests/ldb_match_test.valgrind


Changeset truncated at 500 lines:

diff --git a/lib/ldb/ABI/ldb-1.3.0.sigs b/lib/ldb/ABI/ldb-1.3.8.sigs
similarity index 100%
copy from lib/ldb/ABI/ldb-1.3.0.sigs
copy to lib/ldb/ABI/ldb-1.3.8.sigs
diff --git a/lib/ldb/ABI/pyldb-util-1.1.10.sigs b/lib/ldb/ABI/pyldb-util-1.3.8.sigs
similarity index 100%
copy from lib/ldb/ABI/pyldb-util-1.1.10.sigs
copy to lib/ldb/ABI/pyldb-util-1.3.8.sigs
diff --git a/lib/ldb/ABI/pyldb-util-1.1.10.sigs b/lib/ldb/ABI/pyldb-util.py3-1.3.8.sigs
similarity index 100%
copy from lib/ldb/ABI/pyldb-util-1.1.10.sigs
copy to lib/ldb/ABI/pyldb-util.py3-1.3.8.sigs
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 25fe3f9c21b..829afa77e71 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -244,6 +244,11 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
 	uint8_t *save_p = NULL;
 	unsigned int c = 0;
 
+	if (tree->operation != LDB_OP_SUBSTRING) {
+		*matched = false;
+		return LDB_ERR_INAPPROPRIATE_MATCHING;
+	}
+
 	a = ldb_schema_attribute_by_name(ldb, tree->u.substring.attr);
 	if (!a) {
 		return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
@@ -306,14 +311,38 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
 		p = memmem((const void *)val.data,val.length,
 			   (const void *)cnk.data, cnk.length);
 		if (p == NULL) goto mismatch;
+
+		/*
+		 * At this point we know cnk.length <= val.length as
+		 * otherwise there could be no match
+		 */
+
 		if ( (! tree->u.substring.chunks[c + 1]) && (! tree->u.substring.end_with_wildcard) ) {
 			uint8_t *g;
+			uint8_t *end = val.data + val.length;
 			do { /* greedy */
-				g = memmem(p + cnk.length,
-					val.length - (p - val.data),
-					(const uint8_t *)cnk.data,
-					cnk.length);
-				if (g) p = g;
+
+				/*
+				 * haystack is a valid pointer in val
+				 * because the memmem() can only
+				 * succeed if the needle (cnk.length)
+				 * is <= haystacklen
+				 *
+				 * p will be a pointer at least
+				 * cnk.length from the end of haystack
+				 */
+				uint8_t *haystack
+					= p + cnk.length;
+				size_t haystacklen
+					= end - (haystack);
+
+				g = memmem(haystack,
+					   haystacklen,
+					   (const uint8_t *)cnk.data,
+					   cnk.length);
+				if (g) {
+					p = g;
+				}
 			} while(g);
 		}
 		val.length = val.length - (p - (uint8_t *)(val.data)) - cnk.length;
@@ -324,7 +353,7 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
 	}
 
 	/* last chunk may not have reached end of string */
-	if ( (! tree->u.substring.end_with_wildcard) && (*(val.data) != 0) ) goto mismatch;
+	if ( (! tree->u.substring.end_with_wildcard) && (val.length != 0) ) goto mismatch;
 	talloc_free(save_p);
 	*matched = true;
 	return LDB_SUCCESS;
diff --git a/lib/ldb/common/ldb_parse.c b/lib/ldb/common/ldb_parse.c
index 5fa5a74afa9..db420091311 100644
--- a/lib/ldb/common/ldb_parse.c
+++ b/lib/ldb/common/ldb_parse.c
@@ -389,7 +389,7 @@ static struct ldb_parse_tree *ldb_parse_simple(TALLOC_CTX *mem_ctx, const char *
 	struct ldb_parse_tree *ret;
 	enum ldb_parse_op filtertype;
 
-	ret = talloc(mem_ctx, struct ldb_parse_tree);
+	ret = talloc_zero(mem_ctx, struct ldb_parse_tree);
 	if (!ret) {
 		errno = ENOMEM;
 		return NULL;
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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * from cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ */
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#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 e6b3d4f2ced..a5703ebded4 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 
 APPNAME = 'ldb'
-VERSION = '1.3.7'
+VERSION = '1.3.8'
 
 blddir = 'bin'
 
@@ -364,6 +364,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)
+
 def test(ctx):
     '''run ldb testsuite'''
     import Utils, samba_utils, shutil
@@ -392,7 +397,8 @@ def test(ctx):
 
     cmocka_ret = 0
     for test_exe in ['ldb_tdb_mod_op_test',
-                     'ldb_msg_test']:
+                     'ldb_msg_test',
+                     'ldb_match_test']:
             cmd = os.path.join(Utils.g_module.blddir, test_exe)
             cmocka_ret = cmocka_ret or samba_utils.RUN_COMMAND(cmd)
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list