[SCM] Samba Shared Repository - branch master updated
Gary Lockyer
gary at samba.org
Mon Feb 25 21:55:01 UTC 2019
The branch, master has been updated
via de3bb5cd523 CVE-2019-3824 ldb: Release ldb 1.6.1
via 45b75db50f5 CVE-2019-3824 ldb: Add tests for ldb_wildcard_match
via 42f0f57eb81 CVE-2019-3824 ldb: wildcard_match end of data check
via 34383981a0c CVE-2019-3824 ldb: wildcard_match check tree operation
via 8d34d172092 CVE-2019-3824 ldb: ldb_parse_tree use talloc_zero
via 9427806f729 CVE-2019-3824 ldb: Improve code style and layout in wildcard processing
via 745b99fc6b7 CVE-2019-3824 ldb: Extra comments to clarify no pointer wrap in wildcard processing
via 3674b0891af CVE-2019-3824 ldb: Out of bound read in ldb_wildcard_compare
from e8e9677154c libcli: Pass buf/len to smb2_negotiate_context_add
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit de3bb5cd5236565f2b79644d99e55d03b254b65e
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue Feb 19 10:16:03 2019 +1300
CVE-2019-3824 ldb: Release ldb 1.6.1
* 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: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
Autobuild-User(master): Gary Lockyer <gary at samba.org>
Autobuild-Date(master): Mon Feb 25 22:54:13 CET 2019 on sn-devel-144
commit 45b75db50f5c1a7c8c38af59a62fccee5401c845
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 42f0f57eb819ce6b68a8c5b3b53123b83ec917e3
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 34383981a0c40860f71a4451ff8fd752e1b67666
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 8d34d172092f71baad0d777567e49aebfa07313d
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 9427806f7298d71bd7edfbdda7506ec63f15dda1
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>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
commit 745b99fc6b75db33cdb0a58df1a3f2a5063bc76e
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>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
commit 3674b0891afb016c83763520b87e9f190dcfe884
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
-----------------------------------------------------------------------
Summary of changes:
lib/ldb/ABI/{ldb-1.5.1.sigs => ldb-1.6.1.sigs} | 0
...yldb-util-1.1.10.sigs => pyldb-util-1.6.1.sigs} | 0
...-util-1.1.10.sigs => pyldb-util.py3-1.6.1.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.5.1.sigs => ldb-1.6.1.sigs} (100%)
copy lib/ldb/ABI/{pyldb-util-1.1.10.sigs => pyldb-util-1.6.1.sigs} (100%)
copy lib/ldb/ABI/{pyldb-util-1.1.10.sigs => pyldb-util.py3-1.6.1.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.5.1.sigs b/lib/ldb/ABI/ldb-1.6.1.sigs
similarity index 100%
copy from lib/ldb/ABI/ldb-1.5.1.sigs
copy to lib/ldb/ABI/ldb-1.6.1.sigs
diff --git a/lib/ldb/ABI/pyldb-util-1.1.10.sigs b/lib/ldb/ABI/pyldb-util-1.6.1.sigs
similarity index 100%
copy from lib/ldb/ABI/pyldb-util-1.1.10.sigs
copy to lib/ldb/ABI/pyldb-util-1.6.1.sigs
diff --git a/lib/ldb/ABI/pyldb-util-1.1.10.sigs b/lib/ldb/ABI/pyldb-util.py3-1.6.1.sigs
similarity index 100%
copy from lib/ldb/ABI/pyldb-util-1.1.10.sigs
copy to lib/ldb/ABI/pyldb-util.py3-1.6.1.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 6e224e7b4b7..028b1c4677c 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -1,7 +1,7 @@
#!/usr/bin/env python
APPNAME = 'ldb'
-VERSION = '1.6.0'
+VERSION = '1.6.1'
import sys, os
@@ -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)
--
Samba Shared Repository
More information about the samba-cvs
mailing list