[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Wed Apr 7 03:17:01 UTC 2021


The branch, master has been updated
       via  24ddc1ca9ca ldb/attrib_handler casefold: simplify space dropping
       via  2b2f4f51945 ldb: fix ldb_comparison_fold off-by-one overrun
       via  ff1c3af603b build: Only add -Wl,--as-needed when supported
      from  4d3b6506d30 librpc: Remove the gensec dependency from library dcerpc-binding

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 24ddc1ca9cad95673bdd8023d99867707b37085f
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Tue Dec 8 22:00:55 2020 +1300

    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 at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Wed Apr  7 03:16:39 UTC 2021 on sn-devel-184

commit 2b2f4f519454beb6f2a46705675a62274019fc09
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Sat Mar 6 16:05:15 2021 +1300

    ldb: fix ldb_comparison_fold off-by-one overrun
    
    We run one character over in comparing all the bytes in two ldb_vals.
    
    In almost all circumstances both ldb_vals would have an allocated '\0'
    in the overrun position, but it is best not to rely on that.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit ff1c3af603b47a7e8f9faad8d1c2e4a489559155
Author: Martin Schwenke <martin at meltin.net>
Date:   Mon Mar 29 16:30:37 2021 +1100

    build: Only add -Wl,--as-needed when supported
    
    If -Wl,--as-needed is added to EXTRA_LDFLAGS (via ADD_LDFLAGS, as per
    commit 996560191ac6bd603901dcd6c0de5d239e019ef4) then on some
    platforms (at least CentOS 8 and Fedora 33), any indirect/recursive
    dependencies (i.e. private libraries) are added to both the
    binary (reqid_test in the CTDB case) and to samba-util.so.  However,
    only samba-util.so has rpath set to find private libraries.
    
    When ld.so tries to resolve these dependencies for the binary it
    fails. This may be a bug on those platforms, but it occurs reliably
    and our users will also hit the bug.  For binaries that have other
    private library dependencies (e.g. bundled talloc) rpath will contain
    the private library directory so the duplicate private library
    dependencies are then found... that is, when it works, it works by
    accident!
    
    For some reason (deep in waf or wafsamba) if -Wl,--as-needed is added to
    LINKFLAGS (as is done in conf.add_as_needed()) then it works: the direct
    dependencies are only added to samba-util.so and the same depenencies
    (indirect dependencies for binaries) are not added incorrectly to the
    binaries.
    
    So, without changing 1/2 of waf/wafsamba the simplest fix is to revert
    to adding -Wl,--as-needed to LINKFLAGS, which was the case before
    commit 996560191ac6bd603901dcd6c0de5d239e019ef4.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14288
    
    Signed-off-by: Amitay Isaacs <amitay at gmail.com>
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Bjoern Jacke <bj at sernet.de>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 lib/ldb/common/attrib_handlers.c | 57 +++++++++++++++++++---------------------
 lib/ldb/tests/ldb_match_test.c   |  2 ++
 wscript                          |  4 +--
 3 files changed, 31 insertions(+), 32 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c
index 81a74584bcb..febf2f414ca 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;
 }
 
@@ -335,8 +332,8 @@ int ldb_comparison_fold(struct ldb_context *ldb, void *mem_ctx,
 		if (toupper((unsigned char)*s1) != toupper((unsigned char)*s2))
 			break;
 		if (*s1 == ' ') {
-			while (n1 && s1[0] == s1[1]) { s1++; n1--; }
-			while (n2 && s2[0] == s2[1]) { s2++; n2--; }
+			while (n1 > 1 && s1[0] == s1[1]) { s1++; n1--; }
+			while (n2 > 1 && s2[0] == s2[1]) { s2++; n2--; }
 		}
 		s1++; s2++;
 		n1--; n2--;
diff --git a/lib/ldb/tests/ldb_match_test.c b/lib/ldb/tests/ldb_match_test.c
index ba6ea56be15..1bb56d072d9 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),
diff --git a/wscript b/wscript
index 9c501e9441f..dc547b44aa4 100644
--- a/wscript
+++ b/wscript
@@ -336,8 +336,8 @@ def configure(conf):
     # allows us to find problems on our development hosts faster.
     # It also results in faster load time.
 
-    conf.ADD_LDFLAGS('-Wl,--as-needed', testflags=True)
-
+    if conf.CHECK_LDFLAGS('-Wl,--as-needed'):
+        conf.env.append_unique('LINKFLAGS', '-Wl,--as-needed')
 
     if not conf.CHECK_NEED_LC("-lc not needed"):
         conf.ADD_LDFLAGS('-lc', testflags=False)


-- 
Samba Shared Repository



More information about the samba-cvs mailing list