From 6829a7cd138bf55efa891b70b2264d0a296e5f47 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 3 May 2019 17:27:51 +1200 Subject: [PATCH 1/3] CVE-2019-14847 dsdb/modules/dirsync: ensure attrs exist (CID 1107212) BUG: https://bugzilla.samba.org/show_bug.cgi?id=14040 Signed-off-by: Douglas Bagnall Reviewed-by: Gary Lockyer (cherry picked from commit 23f72c4d712f8d1fec3d67a66d477709d5b0abe2) --- source4/dsdb/samdb/ldb_modules/dirsync.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c index 2a9895ae641..fe9e81f15a3 100644 --- a/source4/dsdb/samdb/ldb_modules/dirsync.c +++ b/source4/dsdb/samdb/ldb_modules/dirsync.c @@ -343,6 +343,10 @@ skip: attr = dsdb_attribute_by_lDAPDisplayName(dsc->schema, el->name); + if (attr == NULL) { + continue; + } + keep = false; if (attr->linkID & 1) { -- 2.11.0 From a1ac8bf2b4ec9fa2dbf4d9a50b08aa61925c561e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 15 Oct 2019 16:28:46 +1300 Subject: [PATCH 2/3] CVE-2019-14847 dsdb: Demonstrate the correct interaction of ranged_results style attributes and dirsync Incremental results are provided by a flag on the dirsync control, not by changing the attribute name. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14040 Signed-off-by: Andrew Bartlett --- selftest/knownfail.d/dirsync | 1 + source4/dsdb/tests/python/dirsync.py | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 selftest/knownfail.d/dirsync diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync new file mode 100644 index 00000000000..bc49fe0d9bb --- /dev/null +++ b/selftest/knownfail.d/dirsync @@ -0,0 +1 @@ +^samba4.ldap.dirsync.python\(ad_dc_ntvfs\).__main__.ExtendedDirsyncTests.test_dirsync_linkedattributes_range\( \ No newline at end of file diff --git a/source4/dsdb/tests/python/dirsync.py b/source4/dsdb/tests/python/dirsync.py index c6a1df5ea43..e177bfbbfdc 100755 --- a/source4/dsdb/tests/python/dirsync.py +++ b/source4/dsdb/tests/python/dirsync.py @@ -28,6 +28,7 @@ from samba.tests.subunitrun import TestProgram, SubunitOptions import samba.getopt as options import base64 +import ldb from ldb import LdbError, SCOPE_BASE from ldb import Message, MessageElement, Dn from ldb import FLAG_MOD_ADD, FLAG_MOD_DELETE @@ -588,6 +589,31 @@ class SimpleDirsyncTests(DirsyncBaseTests): class ExtendedDirsyncTests(SimpleDirsyncTests): + def test_dirsync_linkedattributes_range(self): + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) + res = self.ldb_admin.search(self.base_dn, + attrs=["member;range=1-1"], + expression="(name=Administrators)", + controls=["dirsync:1:0:0"]) + + self.assertTrue(len(res) > 0) + self.assertTrue(res[0].get("member;range=1-1") is None) + self.assertTrue(res[0].get("member") is not None) + self.assertTrue(len(res[0].get("member")) > 0) + + def test_dirsync_linkedattributes_range_user(self): + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) + try: + res = self.ldb_simple.search(self.base_dn, + attrs=["member;range=1-1"], + expression="(name=Administrators)", + controls=["dirsync:1:0:0"]) + except LdbError as e: + (num, _) = e.args + self.assertEquals(num, ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS) + else: + self.fail() + def test_dirsync_linkedattributes(self): flag_incr_linked = 2147483648 self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) -- 2.11.0 From 71b77079a9a778308cdf558e9f2899480fd5e31c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 15 Oct 2019 15:44:34 +1300 Subject: [PATCH 3/3] CVE-2019-14847 dsdb: Correct behaviour of ranged_results when combined with dirsync BUG: https://bugzilla.samba.org/show_bug.cgi?id=14040 Signed-off-by: Andrew Bartlett --- selftest/knownfail.d/dirsync | 1 - source4/dsdb/samdb/ldb_modules/dirsync.c | 11 ++++++----- source4/dsdb/samdb/ldb_modules/ranged_results.c | 25 ++++++++++++++++++++++--- 3 files changed, 28 insertions(+), 9 deletions(-) delete mode 100644 selftest/knownfail.d/dirsync diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync deleted file mode 100644 index bc49fe0d9bb..00000000000 --- a/selftest/knownfail.d/dirsync +++ /dev/null @@ -1 +0,0 @@ -^samba4.ldap.dirsync.python\(ad_dc_ntvfs\).__main__.ExtendedDirsyncTests.test_dirsync_linkedattributes_range\( \ No newline at end of file diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c index fe9e81f15a3..face6790754 100644 --- a/source4/dsdb/samdb/ldb_modules/dirsync.c +++ b/source4/dsdb/samdb/ldb_modules/dirsync.c @@ -998,7 +998,7 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req } /* - * check if there's an extended dn control + * check if there's a dirsync control */ control = ldb_request_get_control(req, LDB_CONTROL_DIRSYNC_OID); if (control == NULL) { @@ -1327,11 +1327,12 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req } /* - * Remove our control from the list of controls + * Mark dirsync control as uncritical (done) + * + * We need this so ranged_results knows how to behave with + * dirsync */ - if (!ldb_save_controls(control, req, NULL)) { - return ldb_operr(ldb); - } + control->critical = false; dsc->schema = dsdb_get_schema(ldb, dsc); /* * At the begining we make the hypothesis that we will return a complete diff --git a/source4/dsdb/samdb/ldb_modules/ranged_results.c b/source4/dsdb/samdb/ldb_modules/ranged_results.c index 13bf3a2d0a9..98438799997 100644 --- a/source4/dsdb/samdb/ldb_modules/ranged_results.c +++ b/source4/dsdb/samdb/ldb_modules/ranged_results.c @@ -35,14 +35,14 @@ struct rr_context { struct ldb_module *module; struct ldb_request *req; + bool dirsync_in_use; }; static struct rr_context *rr_init_context(struct ldb_module *module, struct ldb_request *req) { - struct rr_context *ac; - - ac = talloc_zero(req, struct rr_context); + struct ldb_control *dirsync_control = NULL; + struct rr_context *ac = talloc_zero(req, struct rr_context); if (ac == NULL) { ldb_set_errstring(ldb_module_get_ctx(module), "Out of Memory"); return NULL; @@ -51,6 +51,16 @@ static struct rr_context *rr_init_context(struct ldb_module *module, ac->module = module; ac->req = req; + /* + * check if there's a dirsync control (as there is an + * interaction between these modules) + */ + dirsync_control = ldb_request_get_control(req, + LDB_CONTROL_DIRSYNC_OID); + if (dirsync_control != NULL) { + ac->dirsync_in_use = true; + } + return ac; } @@ -82,6 +92,15 @@ static int rr_search_callback(struct ldb_request *req, struct ldb_reply *ares) ares->response, ares->error); } + if (ac->dirsync_in_use) { + /* + * We return full attribute values when mixed with + * dirsync + */ + return ldb_module_send_entry(ac->req, + ares->message, + ares->controls); + } /* LDB_REPLY_ENTRY */ temp_ctx = talloc_new(ac->req); -- 2.11.0