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.bagn...@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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Wed Apr 7 03:16:39 UTC 2021 on sn-devel-184 commit 2b2f4f519454beb6f2a46705675a62274019fc09 Author: Douglas Bagnall <douglas.bagn...@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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ff1c3af603b47a7e8f9faad8d1c2e4a489559155 Author: Martin Schwenke <mar...@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 <ami...@gmail.com> Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Bjoern Jacke <b...@sernet.de> Reviewed-by: Andrew Bartlett <abart...@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