ldb/attrib_handler casefold: simplify space dropping

As seen in CVE-2021-20277, ldb_handler_fold() has been making mistakes
when collapsing spaces down to a single space.

This patch fixes the way it handles internal spaces (CVE-2021-20277
was about leading spaces), and involves a rewrite of the parsing loop.

The bug has a detailed description of the problem.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14656

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed Apr  7 03:16:39 UTC 2021 on sn-devel-184
(cherry picked from commit  24ddc1ca9cad95673bdd8023d99867707b37085f)

2 files changed, 27 insertions, 28 deletions

diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c
index c6ef5ad477b..f0fd4f50d8d 100644
--- a/lib/ldb/common/attrib_handlers.c
+++ b/lib/ldb/common/attrib_handlers.c
@@ -54,8 +54,8 @@ int ldb_handler_copy(struct ldb_context *ldb, void *mem_ctx,
 int ldb_handler_fold(struct ldb_context *ldb, void *mem_ctx,
 			    const struct ldb_val *in, struct ldb_val *out)
 {
-	char *s, *t;
-	size_t l;
+	char *s, *t, *start;
+	bool in_space;
 
 	if (!in || !out || !(in->data)) {
 		return -1;
@@ -67,36 +67,33 @@ int ldb_handler_fold(struct ldb_context *ldb, void *mem_ctx,
 		return -1;
 	}
 
-	s = (char *)(out->data);
-	
-	/* remove trailing spaces if any */
-	l = strlen(s);
-	while (l > 0 && s[l - 1] == ' ') l--;
-	s[l] = '\0';
-	
-	/* remove leading spaces if any */
-	if (*s == ' ') {
-		for (t = s; *s == ' '; s++, l--) ;
-
-		/* remove leading spaces by moving down the string */
-		memmove(t, s, l);
-
-		s = t;
+	start = (char *)(out->data);
+	in_space = true;
+	t = start;
+	for (s = start; *s != '\0'; s++) {
+		if (*s == ' ') {
+			if (in_space) {
+				/*
+				 * We already have one (or this is the start)
+				 * and we don't want to add more
+				 */
+				continue;
+			}
+			in_space = true;
+		} else {
+			in_space = false;
+		}
+		*t = *s;
+		t++;
 	}
 
-	/* check middle spaces */
-	while ((t = strchr(s, ' ')) != NULL) {
-		for (s = t; *s == ' '; s++) ;
-
-		if ((s - t) > 1) {
-			l = strlen(s);
-
-			/* remove all spaces but one by moving down the string */
-			memmove(t + 1, s, l);
-		}
+	if (in_space && t != start) {
+		/* the loop will have left a single trailing space */
+		t--;
 	}
+	*t = '\0';
 
-	out->length = strlen((char *)out->data);
+	out->length = t - start;
 	return 0;
 }
 
diff --git a/lib/ldb/tests/ldb_match_test.c b/lib/ldb/tests/ldb_match_test.c
index fbf4106fa78..eb5d9fcee20 100644
--- a/lib/ldb/tests/ldb_match_test.c
+++ b/lib/ldb/tests/ldb_match_test.c
@@ -183,6 +183,8 @@ static void test_wildcard_match(void **state)
 	struct wildcard_test tests[] = {
 		TEST_ENTRY("                     1  0", "1*0*", true, true),
 		TEST_ENTRY("                     1  0", "1 *0", true, true),
+		TEST_ENTRY("                     1  0", "*1 0", true, true),
+		TEST_ENTRY("1    0", "*1 0", true, true),
 		TEST_ENTRY("The value.......end", "*end", true, true),
 		TEST_ENTRY("The value.......end", "*fend", false, true),
 		TEST_ENTRY("The value.......end", "*eel", false, true),