The branch, v4-13-stable has been updated via 440b75fda70 VERSION: Disable GIT_SNAPSHOT for the 4.13.6 release. via ef48e861e84 WHATSNEW: Add release notes for Samba 4.13.6. via 56a72e2562a CVE-2020-27840: pytests: move Dn.validate test to ldb via 2193d840045 CVE-2020-27840 ldb_dn: avoid head corruption in ldb_dn_explode via 7924431e7e4 CVE-2020-27840: pytests:segfault: add ldb.Dn validate test via e0901deb314 CVE-2021-20277 ldb/attrib_handlers casefold: stay in bounds via 309b18d53c1 CVE-2021-20277 ldb: Remove tests from ldb_match_test that do not pass via 736cdfad05c CVE-2021-20277 ldb tests: ldb_match tests with extra spaces via 99d849abc3b ldb: add tests for ldb_wildcard_compare via b3f66d56baa VERSION: Bump version up to 4.13.6... from 6df178003a3 VERSION: Disable GIT_SNAPSHOT for the 4.13.5 release.
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-13-stable - Log ----------------------------------------------------------------- commit 440b75fda7092b88b1986759c40f95ed65c2da24 Author: Karolin Seeger <ksee...@samba.org> Date: Fri Mar 19 10:12:15 2021 +0100 VERSION: Disable GIT_SNAPSHOT for the 4.13.6 release. o BUG #14595: CVE-2020-27840: Heap corruption via crafted DN strings. o BUG #14655: CVE-2021-20277: Out of bounds read in AD DC LDAP server. Signed-off-by: Karolin Seeger <ksee...@samba.org> commit ef48e861e8428f4b90e690e4fdad4cb3d893c289 Author: Karolin Seeger <ksee...@samba.org> Date: Fri Mar 19 10:11:37 2021 +0100 WHATSNEW: Add release notes for Samba 4.13.6. Signed-off-by: Karolin Seeger <ksee...@samba.org> commit 56a72e2562a1e7c690ff2500309fce0371c3576d Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Feb 11 16:28:43 2021 +1300 CVE-2020-27840: pytests: move Dn.validate test to ldb We had the test in the Samba Python segfault suite because a) the signal catching infrastructure was there, and b) the ldb tests lack Samba's knownfail mechanism, which allowed us to assert the failure. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14595 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 2193d84004581d68a6e5b5cac76b7c78bdfc4e33 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Dec 11 16:32:25 2020 +1300 CVE-2020-27840 ldb_dn: avoid head corruption in ldb_dn_explode A DN string with lots of trailing space can cause ldb_dn_explode() to put a zero byte in the wrong place in the heap. When a DN string has a value represented with trailing spaces, like this "CN=foo ,DC=bar" the whitespace is supposed to be ignored. We keep track of this in the `t` pointer, which is NULL when we are not walking through trailing spaces, and points to the first space when we are. We are walking with the `p` pointer, writing the value to `d`, and keeping the length in `l`. "CN=foo ,DC= " ==> "foo " ^ ^ ^ t p d --l--- The value is finished when we encounter a comma or the end of the string. If `t` is not NULL at that point, we assume there are trailing spaces and wind `d and `l` back by the correct amount. Then we switch to expecting an attribute name (e.g. "CN"), until we get to an "=", which puts us back into looking for a value. Unfortunately, we forget to immediately tell `t` that we'd finished the last value, we can end up like this: "CN=foo ,DC= " ==> "" ^ ^ ^ t p d l=0 where `p` is pointing to a new value that contains only spaces, while `t` is still referring to the old value. `p` notices the value ends, and we subtract `p - t` from `d`: "CN=foo ,DC= " ==> ? "" ^ ^ ^ t p d l ~= SIZE_MAX - 8 At that point `d` wants to terminate its string with a '\0', but instead it terminates someone else's byte. This does not crash if the number of trailing spaces is small, as `d` will point into a previous value (a copy of "foo" in this example). Corrupting that value will ultimately not matter, as we will soon try to allocate a buffer `l` long, which will be greater than the available memory and the whole operation will fail properly. However, with more spaces, `d` will point into memory before the beginning of the allocated buffer, with the exact offset depending on the length of the earlier attributes and the number of spaces. What about a longer DN with more attributes? For example, "CN=foo ,DC= ,DC=example,DC=com" -- since `d` has moved out of bounds, won't we continue to use it and write more DN values into mystery memory? Fortunately not, because the aforementioned allocation of `l` bytes must happen first, and `l` is now huge. The allocation happens in a talloc_memdup(), which is by default restricted to allocating 256MB. So this allows a person who controls a string parsed by ldb_dn_explode to corrupt heap memory by placing a single zero byte at a chosen offset before the allocated buffer. An LDAP bind request can send a string DN as a username. This DN is necessarily parsed before the password is checked, so an attacker does not need proper credentials. The attacker can easily cause a denial of service and we cannot rule out more subtle attacks. The immediate solution is to reset `t` to NULL when a comma is encountered, indicating that we are no longer looking at trailing whitespace. Found with the help of Honggfuzz. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14595 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 7924431e7e4bbe41ca2224d8a560d504d6fb25f3 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Feb 11 17:05:14 2021 +1300 CVE-2020-27840: pytests:segfault: add ldb.Dn validate test ldb.Dn.validate wraps ldb_dn_explode. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14595 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit e0901deb3148673aad072ca018cc9f9fb1bd5916 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Dec 8 21:32:09 2020 +1300 CVE-2021-20277 ldb/attrib_handlers casefold: stay in bounds For a string that had N spaces at the beginning, we would try to move N bytes beyond the end of the string. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14655 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> (cherry-picked from commit for master) commit 309b18d53c105f9538e8eb0fb7bf7de52f1a5da5 Author: Andrew Bartlett <abart...@samba.org> Date: Fri Mar 12 11:51:56 2021 +1300 CVE-2021-20277 ldb: Remove tests from ldb_match_test that do not pass This reverts some of the backport of 33a95a1e75b85e9795c4490b78ead2162e2a1f47 This is done here rather than squashed in the cherry-pick of the expanded testsuite because it allows this commit to be simply reverted for the backport of bug 14044 if this lands first, or to be dropped if bug 14044 lands first. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14655 Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> commit 736cdfad05c283af805f0ee343163e0d59b72efa Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Mar 5 20:13:01 2021 +1300 CVE-2021-20277 ldb tests: ldb_match tests with extra spaces BUG: https://bugzilla.samba.org/show_bug.cgi?id=14655 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> (cherry-picked from commit for master) commit 99d849abc3be3e2b2f78c2eb15439b6d140d1fa4 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Fri Mar 5 15:47:56 2021 +1300 ldb: add tests for ldb_wildcard_compare BUG: https://bugzilla.samba.org/show_bug.cgi?id=14044 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Björn Jacke <bja...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> (cherry-picked from commit 33a95a1e75b85e9795c4490b78ead2162e2a1f47) commit b3f66d56baaa453f2ca03da381e95faefe0f225d Author: Karolin Seeger <ksee...@samba.org> Date: Tue Mar 9 09:16:21 2021 +0100 VERSION: Bump version up to 4.13.6... and re-enable GIT_SNAPSHOT. Signed-off-by: Karolin Seeger <ksee...@samba.org> (cherry picked from commit b30c0416390ce4151a6bf97ea44e18e9d668e596) ----------------------------------------------------------------------- Summary of changes: VERSION | 2 +- WHATSNEW.txt | 67 ++++++++++++++++++- lib/ldb/common/attrib_handlers.c | 2 +- lib/ldb/common/ldb_dn.c | 1 + lib/ldb/tests/ldb_match_test.c | 138 ++++++++++++++++++++++++++++++++++++--- lib/ldb/tests/python/crash.py | 45 +++++++++++++ lib/ldb/wscript | 1 + 7 files changed, 242 insertions(+), 14 deletions(-) create mode 100644 lib/ldb/tests/python/crash.py Changeset truncated at 500 lines: diff --git a/VERSION b/VERSION index c24df6ba32e..de7894516ff 100644 --- a/VERSION +++ b/VERSION @@ -25,7 +25,7 @@ ######################################################## SAMBA_VERSION_MAJOR=4 SAMBA_VERSION_MINOR=13 -SAMBA_VERSION_RELEASE=5 +SAMBA_VERSION_RELEASE=6 ######################################################## # If a official release has a serious bug # diff --git a/WHATSNEW.txt b/WHATSNEW.txt index 8b8c349eaa5..4083894bcbb 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,3 +1,67 @@ + ============================== + Release Notes for Samba 4.13.6 + March 24, 2021 + ============================== + + +This is a security release in order to address the following defects: + +o CVE-2020-27840: Heap corruption via crafted DN strings. +o CVE-2021-20277: Out of bounds read in AD DC LDAP server. + + +======= +Details +======= + +o CVE-2020-27840: + An anonymous attacker can crash the Samba AD DC LDAP server by sending easily + crafted DNs as part of a bind request. More serious heap corruption is likely + also possible. + +o CVE-2021-20277: + User-controlled LDAP filter strings against the AD DC LDAP server may crash + the LDAP server. + +For more details, please refer to the security advisories. + + +Changes since 4.13.5 +-------------------- + +o Andrew Bartlett <abart...@samba.org> + * BUG 14655: CVE-2021-20277: Fix out of bounds read in ldb_handler_fold. + +o Douglas Bagnall <douglas.bagn...@catalyst.net.nz> + * BUG 14595: CVE-2020-27840: Fix unauthenticated remote heap corruption via + bad DNs. + * BUG 14655: CVE-2021-20277: Fix out of bounds read in ldb_handler_fold. + + +####################################### +Reporting bugs & Development Discussion +####################################### + +Please discuss this release on the samba-technical mailing list or by +joining the #samba-technical IRC channel on irc.freenode.net. + +If you do report problems then please try to send high quality +feedback. If you don't provide vital information to help us track down +the problem then you will probably be ignored. All bug reports should +be filed under the Samba 4.1 and newer product in the project's Bugzilla +database (https://bugzilla.samba.org/). + + +====================================================================== +== Our Code, Our Bugs, Our Responsibility. +== The Samba Team +====================================================================== + + +Release notes for older releases follow: +---------------------------------------- + + ============================== Release Notes for Samba 4.13.5 March 09, 2021 @@ -70,8 +134,7 @@ database (https://bugzilla.samba.org/). ====================================================================== -Release notes for older releases follow: ----------------------------------------- +---------------------------------------------------------------------- ============================== diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c index b5212b73159..c6ef5ad477b 100644 --- a/lib/ldb/common/attrib_handlers.c +++ b/lib/ldb/common/attrib_handlers.c @@ -76,7 +76,7 @@ int ldb_handler_fold(struct ldb_context *ldb, void *mem_ctx, /* remove leading spaces if any */ if (*s == ' ') { - for (t = s; *s == ' '; s++) ; + for (t = s; *s == ' '; s++, l--) ; /* remove leading spaces by moving down the string */ memmove(t, s, l); diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c index 001fcad621f..cce5ad5b2ff 100644 --- a/lib/ldb/common/ldb_dn.c +++ b/lib/ldb/common/ldb_dn.c @@ -570,6 +570,7 @@ static bool ldb_dn_explode(struct ldb_dn *dn) /* trim back */ d -= (p - t); l -= (p - t); + t = NULL; } in_attr = true; diff --git a/lib/ldb/tests/ldb_match_test.c b/lib/ldb/tests/ldb_match_test.c index e09f50c86ba..fbf4106fa78 100644 --- a/lib/ldb/tests/ldb_match_test.c +++ b/lib/ldb/tests/ldb_match_test.c @@ -91,6 +91,33 @@ static int teardown(void **state) return 0; } +static void escape_string(uint8_t *buf, size_t buflen, + const uint8_t *s, size_t len) +{ + size_t i; + size_t j = 0; + for (i = 0; i < len; i++) { + if (j == buflen - 1) { + goto fin; + } + if (s[i] >= 0x20) { + buf[j] = s[i]; + j++; + } else { + if (j >= buflen - 4) { + goto fin; + } + /* utf-8 control char representation */ + buf[j] = 0xE2; + buf[j + 1] = 0x90; + buf[j + 2] = 0x80 + s[i]; + j+= 3; + } + } +fin: + buf[j] = 0; +} + /* * The wild card pattern "attribute=*" is parsed as an LDB_OP_PRESENT operation @@ -122,23 +149,114 @@ static void test_wildcard_match_star(void **state) * Test basic wild card matching * */ +struct wildcard_test { + uint8_t *val; + size_t val_size; + const char *search; + bool should_match; + bool fold; +}; + +/* + * Q: Why this macro rather than plain struct values? + * A: So we can get the size of the const char[] value while it is still a + * true array, not a pointer. + * + * Q: but why not just use strlen? + * A: so values can contain '\0', which we supposedly allow. + */ + +#define TEST_ENTRY(val, search, should_match, fold) \ + { \ + (uint8_t*)discard_const(val), \ + sizeof(val) - 1, \ + search, \ + should_match, \ + fold \ + } + static void test_wildcard_match(void **state) { struct ldbtest_ctx *ctx = *state; - bool matched = false; - - uint8_t value[] = "The value.......end"; - struct ldb_val val = { - .data = value, - .length = (sizeof(value)) + size_t failed = 0; + size_t i; + struct wildcard_test tests[] = { + 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), + TEST_ENTRY("The value.......end", "*d", true, true), + TEST_ENTRY("The value.......end", "*D*", true, true), + TEST_ENTRY("The value.......end", "*e*d*", true, true), + TEST_ENTRY("end", "*e*d*", true, true), + TEST_ENTRY("end", " *e*d*", true, true), + TEST_ENTRY("1.0..0.0.0.0.0.0.0aAaaaAAAAAAA", "*a", true, true), + TEST_ENTRY("1.0.0.0.0.0.0.0.0.0.0aaaa", "*aaaaa", false, true), + TEST_ENTRY("1.0.0.0.0.0.0.0.0.0.0", "*0.0", true, true), + TEST_ENTRY("1.0.0.0.0.0.0.0.0.0", "1*0*0*0*0*0*0*0*0*0", true, + true), + TEST_ENTRY("1.0.0.0.0.0.0.0.0", "1*0*0*0*0*0*0*0*0*0", false, + true), + TEST_ENTRY("1.0.0.0.000.0.0.0.0", "1*0*0*0*0*0*0*0*0*0", true, + true), + TEST_ENTRY("1\n0\r0\t000.0.0.0.0", "1*0*0*0*0*0*0*0*0", true, + true), + /* + * We allow NUL bytes and redundant spaces in non-casefolding + * syntaxes. + */ + TEST_ENTRY(" 1 0", "*1 0", true, false), + TEST_ENTRY(" 1 0", "*1 0", true, false), + TEST_ENTRY("1 0", "*1 0", false, false), + TEST_ENTRY("1\x00 x", "1*x", true, false), + TEST_ENTRY("1\x00 x", "*x", true, false), + TEST_ENTRY("1\x00 x", "*x*", true, false), + TEST_ENTRY("1\x00 x", "* *", true, false), + TEST_ENTRY("1\x00 x", "1*", true, false), + TEST_ENTRY("1\x00 b* x", "1*b*", true, false), + TEST_ENTRY("1.0..0.0.0.0.0.0.0aAaaaAAAAAAA", "*a", false, false), }; - struct ldb_parse_tree *tree = ldb_parse_tree(ctx, "objectClass=*end"); - assert_non_null(tree); - ldb_wildcard_compare(ctx->ldb, tree, val, &matched); - assert_true(matched); + for (i = 0; i < ARRAY_SIZE(tests); i++) { + bool matched; + int ret; + struct ldb_val val = { + .data = (uint8_t *)tests[i].val, + .length = tests[i].val_size + }; + const char *attr = tests[i].fold ? "objectclass" : "birthLocation"; + const char *s = talloc_asprintf(ctx, "%s=%s", + attr, tests[i].search); + struct ldb_parse_tree *tree = ldb_parse_tree(ctx, s); + assert_non_null(tree); + ret = ldb_wildcard_compare(ctx->ldb, tree, val, &matched); + if (ret != LDB_SUCCESS) { + uint8_t buf[100]; + escape_string(buf, sizeof(buf), + tests[i].val, tests[i].val_size); + print_error("%zu val: «%s», search «%s» FAILED with %d\n", + i, buf, tests[i].search, ret); + failed++; + } + if (matched != tests[i].should_match) { + uint8_t buf[100]; + escape_string(buf, sizeof(buf), + tests[i].val, tests[i].val_size); + print_error("%zu val: «%s», search «%s» should %s\n", + i, buf, tests[i].search, + matched ? "not match" : "match"); + failed++; + } + } + if (failed != 0) { + fail_msg("wrong results for %zu/%zu wildcard searches\n", + failed, ARRAY_SIZE(tests)); + } } +#undef TEST_ENTRY + /* * ldb_handler_copy and ldb_val_dup over allocate by one and add a trailing '\0' diff --git a/lib/ldb/tests/python/crash.py b/lib/ldb/tests/python/crash.py new file mode 100644 index 00000000000..32839814552 --- /dev/null +++ b/lib/ldb/tests/python/crash.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python3 +# +# Tests for crashing functions + +import os +from unittest import TestCase +import os +import sys +import traceback + +import ldb + + +def segfault_detector(f): + def wrapper(*args, **kwargs): + pid = os.fork() + if pid == 0: + # child, crashing? + try: + f(*args, **kwargs) + except Exception as e: + traceback.print_exc() + sys.stderr.flush() + sys.stdout.flush() + os._exit(0) + + # parent, waiting + pid2, status = os.waitpid(pid, 0) + if os.WIFSIGNALED(status): + signal = os.WTERMSIG(status) + raise AssertionError("Failed with signal %d" % signal) + + return wrapper + + +class LdbDnCrashTests(TestCase): + @segfault_detector + def test_ldb_dn_explode_crash(self): + for i in range(106, 150): + dn = ldb.Dn(ldb.Ldb(), "a=b%s,c= " % (' ' * i)) + dn.validate() + +if __name__ == '__main__': + import unittest + unittest.TestProgram() diff --git a/lib/ldb/wscript b/lib/ldb/wscript index f374f64aeab..32a1a2e0ec0 100644 --- a/lib/ldb/wscript +++ b/lib/ldb/wscript @@ -614,6 +614,7 @@ def test(ctx): os.mkdir(tmp_dir) pyret = samba_utils.RUN_PYTHON_TESTS( ['tests/python/api.py', + 'tests/python/crash.py', 'tests/python/index.py', 'tests/python/repack.py'], extra_env={'SELFTEST_PREFIX': test_prefix}) -- Samba Shared Repository