Attachment 15427 Details for Bug 13695 – patch for master (v2)

[patch] patch for master (v2)

ldb-filter.patch (text/plain), 30.78 KB, created by Andrew Bartlett on 2019-08-26 04:45:04 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2019-08-26 04:45:04 UTC

Size: 30.78 KB

patch

obsolete

>From b017094a7b6318c2a6144848766c63d706b5aba7 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 26 Aug 2019 13:04:07 +1200
>Subject: [PATCH 1/3] ldb: Correct Pigeonhole principle validation in
> ldb_filter_attrs()
>
>Thankfully this only fails if the DB is corrupt and has a duplicate record.
>
>The test was at the wrong end of the loop, and was for the
>wrong boundary condition.  A write after the end of the array would
>occour before the condition was hit.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13695
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/ldb/common/ldb_pack.c             |  31 +-
> lib/ldb/tests/ldb_filter_attrs_test.c | 986 ++++++++++++++++++++++++++++++++++
> lib/ldb/wscript                       |   5 +
> 3 files changed, 1013 insertions(+), 9 deletions(-)
> create mode 100644 lib/ldb/tests/ldb_filter_attrs_test.c
>
>diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
>index 9d87a10b9f1..1d67622b69e 100644
>--- a/lib/ldb/common/ldb_pack.c
>+++ b/lib/ldb/common/ldb_pack.c
>@@ -1163,7 +1163,10 @@ int ldb_filter_attrs(struct ldb_context *ldb,
> 	} else if (i == 0) {
> 		return 0;
> 
>-	/* Otherwise we are copying at most as many element as we have attributes */
>+	/*
>+	 * Otherwise we are copying at most as many elements as we
>+	 * have attributes
>+	 */
> 	} else {
> 		elements_size = i;
> 	}
>@@ -1177,7 +1180,12 @@ int ldb_filter_attrs(struct ldb_context *ldb,
> 
> 	for (i = 0; i < msg->num_elements; i++) {
> 		struct ldb_message_element *el = &msg->elements[i];
>-		struct ldb_message_element *el2 = &filtered_msg->elements[num_elements];
>+
>+		/*
>+		 * el2 is assigned after the Pigeonhole principle
>+		 * check below for clarity
>+		 */
>+		struct ldb_message_element *el2 = NULL;
> 		unsigned int j;
> 
> 		if (keep_all == false) {
>@@ -1193,6 +1201,18 @@ int ldb_filter_attrs(struct ldb_context *ldb,
> 				continue;
> 			}
> 		}
>+
>+		/*
>+		 * Pigeonhole principle: we can't have more elements
>+		 * than the number of attributes if they are unique in
>+		 * the DB.
>+		 */
>+		if (num_elements >= elements_size) {
>+			goto failed;
>+		}
>+
>+		el2 = &filtered_msg->elements[num_elements];
>+
> 		*el2 = *el;
> 		el2->name = talloc_strdup(filtered_msg->elements,
> 					  el->name);
>@@ -1211,13 +1231,6 @@ int ldb_filter_attrs(struct ldb_context *ldb,
> 			}
> 		}
> 		num_elements++;
>-
>-		/* Pidginhole principle: we can't have more elements
>-		 * than the number of attributes if they are unique in
>-		 * the DB */
>-		if (num_elements > elements_size) {
>-			goto failed;
>-		}
> 	}
> 
> 	filtered_msg->num_elements = num_elements;
>diff --git a/lib/ldb/tests/ldb_filter_attrs_test.c b/lib/ldb/tests/ldb_filter_attrs_test.c
>new file mode 100644
>index 00000000000..d04775879ac
>--- /dev/null
>+++ b/lib/ldb/tests/ldb_filter_attrs_test.c
>@@ -0,0 +1,986 @@
>+/*
>+ * Tests exercising the ldb_filter_attrs().
>+ *
>+ *
>+ * Copyright (C) Catalyst.NET Ltd 2017
>+ * Copyright (C) Andrew Bartlett <abartlet@samba.org> 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/>.
>+ *
>+ */
>+
>+/*
>+ * 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 "../include/ldb.h"
>+#include "../include/ldb_module.h"
>+
>+struct ldbtest_ctx {
>+	struct tevent_context *ev;
>+	struct ldb_context *ldb;
>+};
>+
>+/*
>+ * NOTE WELL:
>+ *
>+ * This test checks the current behaviour of the function, however
>+ * this is not in a public ABI and many of the tested behaviours are
>+ * not ideal.  If the behaviour is deliberatly improved, this test
>+ * should be updated without worry to the new better behaviour.
>+ *
>+ * In particular the test is particularly to ensure the current
>+ * behaviour is memory-safe.
>+ */
>+
>+static int setup(void **state)
>+{
>+	struct ldbtest_ctx *test_ctx;
>+
>+	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);
>+
>+	*state = test_ctx;
>+	return 0;
>+}
>+
>+static int teardown(void **state)
>+{
>+	talloc_free(*state);
>+	return 0;
>+}
>+
>+
>+/*
>+ * Test against a record with only one attribute, matching the one in
>+ * the list
>+ */
>+static void test_filter_attrs_one_attr_matched(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"foo", NULL};
>+
>+	uint8_t value[] = "The value.......end";
>+	struct ldb_val value_1 = {
>+		.data   = value,
>+		.length = (sizeof(value))
>+	};
>+	struct ldb_message_element element_1 = {
>+		.name = "foo",
>+		.num_values = 1,
>+		.values = &value_1
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 1,
>+		.elements = &element_1,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+
>+	/*
>+	 * assert the ldb_filter_attrs does not read or modify
>+	 * filtered_msg.dn in this case
>+	 */
>+	assert_null(filtered_msg->dn);
>+	assert_int_equal(filtered_msg->num_elements, 1);
>+	assert_string_equal(filtered_msg->elements[0].name, "foo");
>+	assert_int_equal(filtered_msg->elements[0].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[0].values[0].length,
>+			 sizeof(value));
>+	assert_memory_equal(filtered_msg->elements[0].values[0].data,
>+			    value, sizeof(value));
>+}
>+
>+/*
>+ * Test against a record with only one attribute, matching the one of
>+ * the multiple attributes in the list
>+ */
>+static void test_filter_attrs_one_attr_matched_of_many(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"foo", "bar", "baz", NULL};
>+
>+	uint8_t value[] = "The value.......end";
>+	struct ldb_val value_1 = {
>+		.data   = value,
>+		.length = (sizeof(value))
>+	};
>+	struct ldb_message_element element_1 = {
>+		.name = "foo",
>+		.num_values = 1,
>+		.values = &value_1
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 1,
>+		.elements = &element_1,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+
>+	/*
>+	 * assert the ldb_filter_attrs does not read or modify
>+	 * filtered_msg.dn in this case
>+	 */
>+	assert_null(filtered_msg->dn);
>+	assert_int_equal(filtered_msg->num_elements, 1);
>+	assert_string_equal(filtered_msg->elements[0].name, "foo");
>+	assert_int_equal(filtered_msg->elements[0].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[0].values[0].length,
>+			 sizeof(value));
>+	assert_memory_equal(filtered_msg->elements[0].values[0].data,
>+			    value, sizeof(value));
>+}
>+
>+/*
>+ * Test against a record with only one attribute, matching both
>+ * attributes in the list
>+ */
>+static void test_filter_attrs_two_attr_matched_attrs(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	/* deliberatly the other order */
>+	const char *attrs[] = {"bar", "foo", NULL};
>+
>+	uint8_t value1[] = "The value.......end";
>+	uint8_t value2[] = "The value..MUST.end";
>+	struct ldb_val value_1 = {
>+		.data   = value1,
>+		.length = (sizeof(value1))
>+	};
>+	struct ldb_val value_2 = {
>+		.data   = value2,
>+		.length = (sizeof(value2))
>+	};
>+
>+	/* foo and bar are the other order to in attrs */
>+	struct ldb_message_element elements[] = {
>+		{
>+			.name = "foo",
>+			.num_values = 1,
>+			.values = &value_1
>+		},
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_2
>+		}
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 2,
>+		.elements = elements,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+	assert_int_equal(filtered_msg->num_elements, 2);
>+
>+	/*
>+	 * assert the ldb_filter_attrs does not read or modify
>+	 * filtered_msg.dn in this case
>+	 */
>+	assert_null(filtered_msg->dn);
>+
>+	/* Assert that DB order is preserved */
>+	assert_string_equal(filtered_msg->elements[0].name, "foo");
>+	assert_int_equal(filtered_msg->elements[0].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[0].values[0].length,
>+			 sizeof(value1));
>+	assert_memory_equal(filtered_msg->elements[0].values[0].data,
>+			    value1, sizeof(value1));
>+	assert_string_equal(filtered_msg->elements[1].name, "bar");
>+	assert_int_equal(filtered_msg->elements[1].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[1].values[0].length,
>+			 sizeof(value2));
>+	assert_memory_equal(filtered_msg->elements[1].values[0].data,
>+			    value2, sizeof(value2));
>+}
>+
>+/*
>+ * Test against a record with two attributes, only of which is in
>+ * the list
>+ */
>+static void test_filter_attrs_two_attr_matched_one_attr(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	/* deliberatly the other order */
>+	const char *attrs[] = {"bar", NULL};
>+
>+	uint8_t value1[] = "The value.......end";
>+	uint8_t value2[] = "The value..MUST.end";
>+	struct ldb_val value_1 = {
>+		.data   = value1,
>+		.length = (sizeof(value1))
>+	};
>+	struct ldb_val value_2 = {
>+		.data   = value2,
>+		.length = (sizeof(value2))
>+	};
>+
>+	/* foo and bar are the other order to in attrs */
>+	struct ldb_message_element elements[] = {
>+		{
>+			.name = "foo",
>+			.num_values = 1,
>+			.values = &value_1
>+		},
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_2
>+		}
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 2,
>+		.elements = elements,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+	assert_int_equal(filtered_msg->num_elements, 1);
>+
>+	/*
>+	 * assert the ldb_filter_attrs does not read or modify
>+	 * filtered_msg.dn in this case
>+	 */
>+	assert_null(filtered_msg->dn);
>+
>+	/* Assert that DB order is preserved */
>+	assert_string_equal(filtered_msg->elements[0].name, "bar");
>+	assert_int_equal(filtered_msg->elements[0].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[0].values[0].length,
>+			 sizeof(value2));
>+	assert_memory_equal(filtered_msg->elements[0].values[0].data,
>+			    value2, sizeof(value2));
>+}
>+
>+/*
>+ * Test against a record with two attributes, both matching the one
>+ * specified attribute in the list (a corrupt record)
>+ */
>+static void test_filter_attrs_two_dup_attr_matched_one_attr(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	/* deliberatly the other order */
>+	const char *attrs[] = {"bar", NULL};
>+
>+	uint8_t value1[] = "The value.......end";
>+	uint8_t value2[] = "The value..MUST.end";
>+	struct ldb_val value_1 = {
>+		.data   = value1,
>+		.length = (sizeof(value1))
>+	};
>+	struct ldb_val value_2 = {
>+		.data   = value2,
>+		.length = (sizeof(value2))
>+	};
>+
>+	/* foo and bar are the other order to in attrs */
>+	struct ldb_message_element elements[] = {
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_1
>+		},
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_2
>+		}
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 2,
>+		.elements = elements,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+
>+	/* This should fail the pidgenhole test */
>+	assert_int_equal(ret, -1);
>+}
>+
>+/*
>+ * Test against a record with two attributes, both matching the one
>+ * specified attribute in the list (a corrupt record)
>+ */
>+static void test_filter_attrs_two_dup_attr_matched_dup(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"bar", "bar", NULL};
>+
>+	uint8_t value1[] = "The value.......end";
>+	uint8_t value2[] = "The value..MUST.end";
>+	struct ldb_val value_1 = {
>+		.data   = value1,
>+		.length = (sizeof(value1))
>+	};
>+	struct ldb_val value_2 = {
>+		.data   = value2,
>+		.length = (sizeof(value2))
>+	};
>+
>+	/* foo and bar are the other order to in attrs */
>+	struct ldb_message_element elements[] = {
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_1
>+		},
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_2
>+		}
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 2,
>+		.elements = elements,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+
>+	/* This does not fail the pidgenhole test */
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_int_equal(filtered_msg->num_elements, 2);
>+
>+	/* Assert that DB order is preserved */
>+	assert_string_equal(filtered_msg->elements[0].name, "bar");
>+	assert_int_equal(filtered_msg->elements[0].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[0].values[0].length,
>+			 sizeof(value1));
>+	assert_memory_equal(filtered_msg->elements[0].values[0].data,
>+			    value1, sizeof(value1));
>+	assert_string_equal(filtered_msg->elements[1].name, "bar");
>+	assert_int_equal(filtered_msg->elements[1].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[1].values[0].length,
>+			 sizeof(value2));
>+	assert_memory_equal(filtered_msg->elements[1].values[0].data,
>+			    value2, sizeof(value2));
>+}
>+
>+/*
>+ * Test against a record with two attributes, both matching one of the
>+ * specified attributes in the list (a corrupt record)
>+ */
>+static void test_filter_attrs_two_dup_attr_matched_one_of_two(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"bar", "foo", NULL};
>+
>+	uint8_t value1[] = "The value.......end";
>+	uint8_t value2[] = "The value..MUST.end";
>+	struct ldb_val value_1 = {
>+		.data   = value1,
>+		.length = (sizeof(value1))
>+	};
>+	struct ldb_val value_2 = {
>+		.data   = value2,
>+		.length = (sizeof(value2))
>+	};
>+
>+	/* foo and bar are the other order to in attrs */
>+	struct ldb_message_element elements[] = {
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_1
>+		},
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_2
>+		}
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 2,
>+		.elements = elements,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+
>+	/* This does not fail the pidgenhole test */
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_int_equal(filtered_msg->num_elements, 2);
>+
>+	/* Assert that DB order is preserved */
>+	assert_string_equal(filtered_msg->elements[0].name, "bar");
>+	assert_int_equal(filtered_msg->elements[0].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[0].values[0].length,
>+			 sizeof(value1));
>+	assert_memory_equal(filtered_msg->elements[0].values[0].data,
>+			    value1, sizeof(value1));
>+	assert_string_equal(filtered_msg->elements[1].name, "bar");
>+	assert_int_equal(filtered_msg->elements[1].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[1].values[0].length,
>+			 sizeof(value2));
>+	assert_memory_equal(filtered_msg->elements[1].values[0].data,
>+			    value2, sizeof(value2));
>+}
>+
>+/*
>+ * Test against a record with two attributes against * (but not the
>+ * other named attribute) (a corrupt record)
>+ */
>+static void test_filter_attrs_two_dup_attr_matched_star(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"*", "foo", NULL};
>+
>+	uint8_t value1[] = "The value.......end";
>+	uint8_t value2[] = "The value..MUST.end";
>+	struct ldb_val value_1 = {
>+		.data   = value1,
>+		.length = (sizeof(value1))
>+	};
>+	struct ldb_val value_2 = {
>+		.data   = value2,
>+		.length = (sizeof(value2))
>+	};
>+
>+	/* foo and bar are the other order to in attrs */
>+	struct ldb_message_element elements[] = {
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_1
>+		},
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_2
>+		}
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 2,
>+		.elements = elements,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	/* Needed as * implies distinguishedName */
>+	filtered_msg->dn = in.dn;
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+
>+	/* This does not fail the pidgenhole test */
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_int_equal(filtered_msg->num_elements, 3);
>+
>+	/* Assert that DB order is preserved */
>+	assert_string_equal(filtered_msg->elements[0].name, "bar");
>+	assert_int_equal(filtered_msg->elements[0].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[0].values[0].length,
>+			 sizeof(value1));
>+	assert_memory_equal(filtered_msg->elements[0].values[0].data,
>+			    value1, sizeof(value1));
>+	assert_string_equal(filtered_msg->elements[1].name, "bar");
>+	assert_int_equal(filtered_msg->elements[1].num_values, 1);
>+	assert_int_equal(filtered_msg->elements[1].values[0].length,
>+			 sizeof(value2));
>+	assert_memory_equal(filtered_msg->elements[1].values[0].data,
>+			    value2, sizeof(value2));
>+	/*
>+	 * assert the ldb_filter_attrs does not modify filtered_msg.dn
>+	 * in this case
>+	 */
>+	assert_ptr_equal(filtered_msg->dn, in.dn);
>+	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
>+							"distinguishedName",
>+							NULL),
>+			    ldb_dn_get_linearized(in.dn));
>+}
>+
>+/*
>+ * Test against a record with only one attribute, matching the * in
>+ * the list
>+ */
>+static void test_filter_attrs_one_attr_matched_star(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"*", NULL};
>+
>+	uint8_t value[] = "The value.......end";
>+	struct ldb_val value_1 = {
>+		.data   = value,
>+		.length = (sizeof(value))
>+	};
>+	struct ldb_message_element element_1 = {
>+		.name = "foo",
>+		.num_values = 1,
>+		.values = &value_1
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 1,
>+		.elements = &element_1,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	/* Needed as * implies distinguishedName */
>+	filtered_msg->dn = in.dn;
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+	assert_int_equal(filtered_msg->num_elements, 2);
>+
>+	/*
>+	 * assert the ldb_filter_attrs does not modify filtered_msg.dn
>+	 * in this case
>+	 */
>+	assert_ptr_equal(filtered_msg->dn, in.dn);
>+	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
>+							"distinguishedName",
>+							NULL),
>+			    ldb_dn_get_linearized(in.dn));
>+	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
>+							"foo",
>+							NULL),
>+			    value);
>+}
>+
>+/*
>+ * Test against a record with two attributes, matching the * in
>+ * the list
>+ */
>+static void test_filter_attrs_two_attr_matched_star(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"*", NULL};
>+
>+	uint8_t value1[] = "The value.......end";
>+	uint8_t value2[] = "The value..MUST.end";
>+	struct ldb_val value_1 = {
>+		.data   = value1,
>+		.length = (sizeof(value1))
>+	};
>+	struct ldb_val value_2 = {
>+		.data   = value2,
>+		.length = (sizeof(value2))
>+	};
>+	struct ldb_message_element elements[] = {
>+		{
>+			.name = "foo",
>+			.num_values = 1,
>+			.values = &value_1
>+		},
>+		{
>+			.name = "bar",
>+			.num_values = 1,
>+			.values = &value_2
>+		}
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 2,
>+		.elements = elements,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	/* Needed as * implies distinguishedName */
>+	filtered_msg->dn = in.dn;
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+	assert_int_equal(filtered_msg->num_elements, 3);
>+
>+	/*
>+	 * assert the ldb_filter_attrs does not modify filtered_msg.dn
>+	 * in this case
>+	 */
>+	assert_ptr_equal(filtered_msg->dn, in.dn);
>+	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
>+							"distinguishedName",
>+							NULL),
>+			    ldb_dn_get_linearized(in.dn));
>+	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
>+							"foo",
>+							NULL),
>+			    value1);
>+	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
>+							"bar",
>+							NULL),
>+			    value2);
>+}
>+
>+/*
>+ * Test against a record with only one attribute, matching the * in
>+ * the list, but without the DN being pre-filled.  Fails due to need
>+ * to contstruct the distinguishedName
>+ */
>+static void test_filter_attrs_one_attr_matched_star_no_dn(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"*", NULL};
>+
>+	uint8_t value[] = "The value.......end";
>+	struct ldb_val value_1 = {
>+		.data   = value,
>+		.length = (sizeof(value))
>+	};
>+	struct ldb_message_element element_1 = {
>+		.name = "foo",
>+		.num_values = 1,
>+		.values = &value_1
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 1,
>+		.elements = &element_1,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, -1);
>+}
>+
>+/*
>+ * Test against a record with only one attribute, matching the * in
>+ * the list plus requsesting distinguishedName
>+ */
>+static void test_filter_attrs_one_attr_matched_star_dn(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"*", "distinguishedName", NULL};
>+
>+	uint8_t value[] = "The value.......end";
>+	struct ldb_val value_1 = {
>+		.data   = value,
>+		.length = (sizeof(value))
>+	};
>+	struct ldb_message_element element_1 = {
>+		.name = "foo",
>+		.num_values = 1,
>+		.values = &value_1
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 1,
>+		.elements = &element_1,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	/* Needed for distinguishedName */
>+	filtered_msg->dn = in.dn;
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+	assert_int_equal(filtered_msg->num_elements, 2);
>+
>+	/* show that ldb_filter_attrs does not modify in.dn */
>+	assert_ptr_equal(filtered_msg->dn, in.dn);
>+
>+	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
>+							"distinguishedName",
>+							NULL),
>+			    ldb_dn_get_linearized(in.dn));
>+	assert_string_equal(ldb_msg_find_attr_as_string(filtered_msg,
>+							"foo",
>+							NULL),
>+			    value);
>+}
>+
>+/*
>+ * Test against a record with only one attribute, but returning
>+ * distinguishedName from the list (only)
>+ */
>+static void test_filter_attrs_one_attr_matched_dn(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {"distinguishedName", NULL};
>+
>+	uint8_t value[] = "The value.......end";
>+	struct ldb_val value_1 = {
>+		.data   = value,
>+		.length = (sizeof(value))
>+	};
>+	struct ldb_message_element element_1 = {
>+		.name = "foo",
>+		.num_values = 1,
>+		.values = &value_1
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 1,
>+		.elements = &element_1,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	/* Needed for distinguishedName */
>+	filtered_msg->dn = in.dn;
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+	assert_int_equal(filtered_msg->num_elements, 1);
>+
>+	/* show that ldb_filter_attrs does not modify in.dn */
>+	assert_ptr_equal(filtered_msg->dn, in.dn);
>+	assert_string_equal(filtered_msg->elements[0].name, "distinguishedName");
>+	assert_int_equal(filtered_msg->elements[0].num_values, 1);
>+	assert_string_equal(filtered_msg->elements[0].values[0].data,
>+			    ldb_dn_get_linearized(in.dn));
>+}
>+
>+/*
>+ * Test against a record with only one attribute, not matching the
>+ * empty attribute list
>+ */
>+static void test_filter_attrs_one_attr_empty_list(void **state)
>+{
>+	struct ldbtest_ctx *ctx = *state;
>+	int ret;
>+
>+	struct ldb_message *filtered_msg = ldb_msg_new(ctx);
>+
>+	const char *attrs[] = {NULL};
>+
>+	uint8_t value[] = "The value.......end";
>+	struct ldb_val value_1 = {
>+		.data   = value,
>+		.length = (sizeof(value))
>+	};
>+	struct ldb_message_element element_1 = {
>+		.name = "foo",
>+		.num_values = 1,
>+		.values = &value_1
>+	};
>+	struct ldb_message in = {
>+		.dn = ldb_dn_new(ctx, ctx->ldb, "dc=samba,dc=org"),
>+		.num_elements = 1,
>+		.elements = &element_1,
>+	};
>+
>+	assert_non_null(in.dn);
>+
>+	ret = ldb_filter_attrs(ctx->ldb,
>+			       &in,
>+			       attrs,
>+			       filtered_msg);
>+	assert_int_equal(ret, LDB_SUCCESS);
>+	assert_non_null(filtered_msg);
>+	assert_int_equal(filtered_msg->num_elements, 0);
>+	assert_null(filtered_msg->dn);
>+	assert_null(filtered_msg->elements);
>+}
>+
>+int main(int argc, const char **argv)
>+{
>+	const struct CMUnitTest tests[] = {
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_one_attr_matched,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_one_attr_matched_of_many,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_two_attr_matched_attrs,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_two_attr_matched_one_attr,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_two_dup_attr_matched_one_attr,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_two_dup_attr_matched_dup,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_two_dup_attr_matched_one_of_two,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_two_dup_attr_matched_star,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_one_attr_matched_star,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_two_attr_matched_star,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_one_attr_matched_star_no_dn,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_one_attr_matched_star_dn,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_one_attr_matched_dn,
>+			setup,
>+			teardown),
>+		cmocka_unit_test_setup_teardown(
>+			test_filter_attrs_one_attr_empty_list,
>+			setup,
>+			teardown),
>+	};
>+
>+	return cmocka_run_group_tests(tests, NULL, NULL);
>+}
>diff --git a/lib/ldb/wscript b/lib/ldb/wscript
>index ea53c60b15b..4c906d2aa77 100644
>--- a/lib/ldb/wscript
>+++ b/lib/ldb/wscript
>@@ -512,6 +512,11 @@ def build(bld):
>                          deps='cmocka ldb ldb_tdb_err_map',
>                          install=False)
> 
>+        bld.SAMBA_BINARY('ldb_filter_attrs_test',
>+                         source='tests/ldb_filter_attrs_test.c',
>+                         deps='cmocka ldb ldb_tdb_err_map',
>+                         install=False)
>+
>         bld.SAMBA_BINARY('ldb_key_value_sub_txn_tdb_test',
>                          bld.SUBDIR('ldb_key_value',
>                              '''ldb_kv_search.c
>-- 
>2.11.0
>
>
>From 288a05e8d23bcbd1ec051e540de7241b611ed18d Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 26 Aug 2019 14:48:52 +1200
>Subject: [PATCH 2/3] ldb: use TALLOC_FREE() over talloc_free() in
> ldb_filter_attrs()
>
>This is a macro that sets the pointer to NULL after the talloc_free()
>and is part of our standard coding practices.
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/ldb/common/ldb_pack.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
>index 1d67622b69e..b5e1fbbc4ff 100644
>--- a/lib/ldb/common/ldb_pack.c
>+++ b/lib/ldb/common/ldb_pack.c
>@@ -1251,8 +1251,7 @@ int ldb_filter_attrs(struct ldb_context *ldb,
> 			goto failed;
> 		}
> 	} else {
>-		talloc_free(filtered_msg->elements);
>-		filtered_msg->elements = NULL;
>+		TALLOC_FREE(filtered_msg->elements);
> 	}
> 
> 	return 0;
>-- 
>2.11.0
>
>
>From 6775965f2053cbdfdc1016206ab756103ebfcac8 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 26 Aug 2019 14:50:15 +1200
>Subject: [PATCH 3/3] ldb: Call TALLOC_FREE(filtered_msg->elements) on
> ldb_filter_attrs() failure
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/ldb/common/ldb_pack.c             | 1 +
> lib/ldb/tests/ldb_filter_attrs_test.c | 2 ++
> 2 files changed, 3 insertions(+)
>
>diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
>index b5e1fbbc4ff..e7dd364008a 100644
>--- a/lib/ldb/common/ldb_pack.c
>+++ b/lib/ldb/common/ldb_pack.c
>@@ -1256,5 +1256,6 @@ int ldb_filter_attrs(struct ldb_context *ldb,
> 
> 	return 0;
> failed:
>+	TALLOC_FREE(filtered_msg->elements);
> 	return -1;
> }
>diff --git a/lib/ldb/tests/ldb_filter_attrs_test.c b/lib/ldb/tests/ldb_filter_attrs_test.c
>index d04775879ac..7d555e0da2e 100644
>--- a/lib/ldb/tests/ldb_filter_attrs_test.c
>+++ b/lib/ldb/tests/ldb_filter_attrs_test.c
>@@ -384,6 +384,7 @@ static void test_filter_attrs_two_dup_attr_matched_one_attr(void **state)
> 
> 	/* This should fail the pidgenhole test */
> 	assert_int_equal(ret, -1);
>+	assert_null(filtered_msg->elements);
> }
> 
> /*
>@@ -772,6 +773,7 @@ static void test_filter_attrs_one_attr_matched_star_no_dn(void **state)
> 			       attrs,
> 			       filtered_msg);
> 	assert_int_equal(ret, -1);
>+	assert_null(filtered_msg->elements);
> }
> 
> /*
>-- 
>2.11.0
>

Actions: View

Attachments on bug 13695: 14703 | 15425 | 15427