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)

diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c
index c6ef5ad477b018a6c3c4cdab187c98e8c2389f24..f0fd4f50d8dfd09bbd132a3a887b46eb182cac4f 100644 (file)
--- 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 fbf4106fa78612aacf610a87bba4b78bad8f7eaa..eb5d9fcee200f1607591e1e98417318ec1b7cab8 100644 (file)
--- 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),