Attachment 15075 Details for Bug 13900 – a fix (with test!)

[patch] a fix (with test!)

hansi-ldb-parse-tree.patch (text/plain), 4.67 KB, created by Douglas Bagnall on 2019-04-16 22:33:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Douglas Bagnall

Created: 2019-04-16 22:33:51 UTC

Size: 4.67 KB

patch

obsolete

>From 9b00904e4a0204b3b31271d3b8c8287c244bfdbc Mon Sep 17 00:00:00 2001
>From: Michael Hanselmann <public@hansmi.ch>
>Date: Fri, 12 Apr 2019 00:46:37 +0200
>Subject: [PATCH] ldb: Avoid read beyond buffer
>
>Calling the "ldb_parse_tree" function with a filter consisting of
>exactly a single space (" ") would trigger a read beyond the input
>buffer. A unittest is included.
>
>Signed-off-by: Michael Hanselmann <public@hansmi.ch>
>---
> lib/ldb/common/ldb_parse.c     |  6 +--
> lib/ldb/tests/ldb_parse_test.c | 93 ++++++++++++++++++++++++++++++++++
> lib/ldb/wscript                |  5 ++
> 3 files changed, 101 insertions(+), 3 deletions(-)
> create mode 100644 lib/ldb/tests/ldb_parse_test.c
>
>diff --git a/lib/ldb/common/ldb_parse.c b/lib/ldb/common/ldb_parse.c
>index db420091311..452c5830ed5 100644
>--- a/lib/ldb/common/ldb_parse.c
>+++ b/lib/ldb/common/ldb_parse.c
>@@ -328,7 +328,7 @@ static enum ldb_parse_op ldb_parse_filtertype(TALLOC_CTX *mem_ctx, char **type,
> 
> 	if (*p == '=') {
> 		filter = LDB_OP_EQUALITY;
>-	} else if (*(p + 1) == '=') {
>+	} else if (*p != '\0' && *(p + 1) == '=') {
> 		switch (*p) {
> 		case '<':
> 			filter = LDB_OP_LESS;
>@@ -679,12 +679,12 @@ static struct ldb_parse_tree *ldb_parse_filter(TALLOC_CTX *mem_ctx, const char *
> */
> struct ldb_parse_tree *ldb_parse_tree(TALLOC_CTX *mem_ctx, const char *s)
> {
>+	while (s && isspace((unsigned char)*s)) s++;
>+
> 	if (s == NULL || *s == 0) {
> 		s = "(|(objectClass=*)(distinguishedName=*))";
> 	}
> 
>-	while (isspace((unsigned char)*s)) s++;
>-
> 	if (*s == '(') {
> 		return ldb_parse_filter(mem_ctx, &s);
> 	}
>diff --git a/lib/ldb/tests/ldb_parse_test.c b/lib/ldb/tests/ldb_parse_test.c
>new file mode 100644
>index 00000000000..a739d7795d1
>--- /dev/null
>+++ b/lib/ldb/tests/ldb_parse_test.c
>@@ -0,0 +1,93 @@
>+/*
>+ * Tests exercising the ldb parse operations.
>+ *
>+ * Copyright (C) Catalyst.NET Ltd 2017
>+ * Copyright (C) Michael Hanselmann 2019
>+ *
>+ * 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/>.
>+ *
>+ */
>+
>+#include <stdarg.h>
>+#include <stddef.h>
>+#include <stdint.h>
>+#include <setjmp.h>
>+#include <cmocka.h>
>+
>+#include "../include/ldb.h"
>+
>+struct test_ctx
>+{
>+};
>+
>+static int setup(void **state)
>+{
>+	struct test_ctx *ctx;
>+
>+	ctx = talloc_zero(NULL, struct test_ctx);
>+	assert_non_null(ctx);
>+
>+	*state = ctx;
>+
>+	return 0;
>+}
>+
>+static int teardown(void **state)
>+{
>+	struct test_ctx *ctx =
>+		talloc_get_type_abort(*state, struct test_ctx);
>+
>+	talloc_free(ctx);
>+
>+	return 0;
>+}
>+
>+static void test_roundtrip(TALLOC_CTX *mem_ctx, const char *filter, const char *expected)
>+{
>+	struct ldb_parse_tree *tree;
>+	char *serialized;
>+
>+	assert_non_null(filter);
>+	assert_non_null(expected);
>+
>+	tree = ldb_parse_tree(mem_ctx, filter);
>+	assert_non_null(tree);
>+
>+	serialized = ldb_filter_from_tree(mem_ctx, tree);
>+	assert_non_null(serialized);
>+
>+	assert_string_equal(serialized, expected);
>+}
>+
>+static void test_parse_filtertype(void **state)
>+{
>+	struct test_ctx *ctx =
>+		talloc_get_type_abort(*state, struct test_ctx);
>+
>+	test_roundtrip(ctx, "", "(|(objectClass=*)(distinguishedName=*))");
>+	test_roundtrip(ctx, "a=value", "(a=value)");
>+	test_roundtrip(ctx, "(|(foo=bar)(baz=hello))", "(|(foo=bar)(baz=hello))");
>+	test_roundtrip(ctx, " ", "(|(objectClass=*)(distinguishedName=*))");
>+}
>+
>+int main(int argc, const char **argv)
>+{
>+	const struct CMUnitTest tests[] = {
>+		cmocka_unit_test_setup_teardown(test_parse_filtertype, setup, teardown),
>+	};
>+
>+	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
>+
>+	return cmocka_run_group_tests(tests, NULL, NULL);
>+}
>diff --git a/lib/ldb/wscript b/lib/ldb/wscript
>index 2d37d4f5a9f..a1c33f40ac7 100644
>--- a/lib/ldb/wscript
>+++ b/lib/ldb/wscript
>@@ -506,6 +506,11 @@ def build(bld):
>                          deps='cmocka ldb ldb_tdb_err_map',
>                          install=False)
> 
>+        bld.SAMBA_BINARY('ldb_parse_test',
>+                         source='tests/ldb_parse_test.c',
>+                         deps='cmocka ldb ldb_tdb_err_map',
>+                         install=False)
>+
>         if bld.CONFIG_SET('HAVE_LMDB'):
>             bld.SAMBA_BINARY('ldb_mdb_mod_op_test',
>                              source='tests/ldb_mod_op_test.c',
>-- 
>2.18.1
>

Actions: View

Attachments on bug 13900: 15075 | 15087