The branch, v4-14-stable has been updated via ed4a04eca53 VERSION: Disable GIT_SNAPSHOT for the 4.14.1 release. via 94b42a3a393 WHATSNEW: Add release notes for Samba 4.14.1. via 2d82f0e1b84 CVE-2020-27840: pytests: move Dn.validate test to ldb via f89767bea73 CVE-2020-27840 ldb_dn: avoid head corruption in ldb_dn_explode via c82bea2b723 CVE-2020-27840: pytests:segfault: add ldb.Dn validate test via fab6b79b772 CVE-2021-20277 ldb/attrib_handlers casefold: stay in bounds via 50e44877c3d CVE-2021-20277 ldb: Remove tests from ldb_match_test that do not pass via 1d966cb12e7 CVE-2021-20277 ldb tests: ldb_match tests with extra spaces via ff12bd2fa12 ldb: add tests for ldb_wildcard_compare via 72ca2fb73a9 VERSION: Bump version up to 4.14.1... from 9b49519cae3 VERSION: Bump version up to 4.14.0...
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-14-stable - Log ----------------------------------------------------------------- commit ed4a04eca53906ab7d69667545b414fa84fe5404 Author: Karolin Seeger <ksee...@samba.org> Date: Tue Mar 23 09:29:08 2021 +0100 VERSION: Disable GIT_SNAPSHOT for the 4.14.1 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 94b42a3a3932169a68b5efccbff2acf7d6464805 Author: Karolin Seeger <ksee...@samba.org> Date: Tue Mar 23 09:28:00 2021 +0100 WHATSNEW: Add release notes for Samba 4.14.1. Signed-off-by: Karolin Seeger <ksee...@samba.org> commit 2d82f0e1b84bb390dbf6a3547e4234bfec4eac21 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 f89767bea7330ec1936d2312e2b1da7b435c04b7 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 c82bea2b723b55dca626fad9f9478d89c90cfd10 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 fab6b79b7724f0b636963be528483e3e946884aa 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 50e44877c3df8098658bd4b1fdad25b8aaadf6f3 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 1d966cb12e7882f9cfb230195e4eff3de0f4e135 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 ff12bd2fa126d105c2fbc62529d664324e22aeb3 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 72ca2fb73a93ef251bbc51241ab92ef205d05f69 Author: Karolin Seeger <ksee...@samba.org> Date: Tue Mar 9 13:31:47 2021 +0100 VERSION: Bump version up to 4.14.1... and-re-enable GIT_SNAPSHOT. Signed-off-by: Karolin Seeger <ksee...@samba.org> (cherry picked from commit 3fa3608e8f00df81ae7504f26459b42da069d322) ----------------------------------------------------------------------- Summary of changes: VERSION | 2 +- WHATSNEW.txt | 64 ++++++++++++++++++ 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, 241 insertions(+), 12 deletions(-) create mode 100644 lib/ldb/tests/python/crash.py Changeset truncated at 500 lines: diff --git a/VERSION b/VERSION index 8ca6b68551c..01b91528239 100644 --- a/VERSION +++ b/VERSION @@ -25,7 +25,7 @@ ######################################################## SAMBA_VERSION_MAJOR=4 SAMBA_VERSION_MINOR=14 -SAMBA_VERSION_RELEASE=0 +SAMBA_VERSION_RELEASE=1 ######################################################## # If a official release has a serious bug # diff --git a/WHATSNEW.txt b/WHATSNEW.txt index 7e46022b2b9..3a0a4589fbf 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,3 +1,67 @@ + ============================== + Release Notes for Samba 4.14.1 + 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.14.0 +-------------------- + +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.14.0 March 09, 2021 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