The Samba-Bugzilla – Attachment 14208 Details for
Bug 13374
[SECURITY] CVE-2018-1140 ldbsearch '(distinguishedName=abc)' and DNS query with escapes crashes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
backported patch for 4.8
ldb-dn-segfault-4.8.patch.txt (text/plain), 16.19 KB, created by
Andrew Bartlett
on 2018-05-21 04:00:58 UTC
(
hide
)
Description:
backported patch for 4.8
Filename:
MIME Type:
Creator:
Andrew Bartlett
Created:
2018-05-21 04:00:58 UTC
Size:
16.19 KB
patch
obsolete
>From 070134e907f05abe47437543da4437c29ce575c0 Mon Sep 17 00:00:00 2001 >From: Andrej Gessel <Andrej.Gessel@janztec.com> >Date: Fri, 6 Apr 2018 18:18:33 +0200 >Subject: [PATCH 1/6] Add NULL check for ldb_dn_get_casefold() in > ltdb_index_dn_attr() > >Signed-off-by: Andrej Gessel <Andrej.Gessel@janztec.com> >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13374 >--- > lib/ldb/ldb_tdb/ldb_index.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > >diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c >index ee2027319e3..cf4363f13ac 100644 >--- a/lib/ldb/ldb_tdb/ldb_index.c >+++ b/lib/ldb/ldb_tdb/ldb_index.c >@@ -1391,6 +1391,15 @@ static int ltdb_index_dn_attr(struct ldb_module *module, > > /* work out the index key from the parent DN */ > val.data = (uint8_t *)((uintptr_t)ldb_dn_get_casefold(dn)); >+ if (val.data == NULL) { >+ const char *dn_str = ldb_dn_get_linearized(dn); >+ ldb_asprintf_errstring(ldb_module_get_ctx(module), >+ __location__ >+ ": Failed to get casefold DN" >+ "from: %s", >+ dn_str); >+ return LDB_ERR_OPERATIONS_ERROR; >+ } > val.length = strlen((char *)val.data); > key = ltdb_index_key(ldb, ltdb, attr, &val, NULL); > if (!key) { >-- >2.11.0 > > >From dd05bbdf054edc3de7c990efa9e00ffeef48b723 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 21 May 2018 14:50:50 +1200 >Subject: [PATCH 2/6] ldb: Check for ldb_dn_get_casefold() failure in > ldb_sqlite > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13374 >--- > lib/ldb/ldb_sqlite3/ldb_sqlite3.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/lib/ldb/ldb_sqlite3/ldb_sqlite3.c b/lib/ldb/ldb_sqlite3/ldb_sqlite3.c >index f94dc993904..0f5abf87547 100644 >--- a/lib/ldb/ldb_sqlite3/ldb_sqlite3.c >+++ b/lib/ldb/ldb_sqlite3/ldb_sqlite3.c >@@ -323,6 +323,9 @@ static char *parsetree_to_sql(struct ldb_module *module, > const char *cdn = ldb_dn_get_casefold( > ldb_dn_new(mem_ctx, ldb, > (const char *)value.data)); >+ if (cdn == NULL) { >+ return NULL; >+ } > > return lsqlite3_tprintf(mem_ctx, > "SELECT eid FROM ldb_entry " >-- >2.11.0 > > >From 8babea7af039b54a372dee399e200480f02e56ae Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 21 May 2018 15:20:26 +1200 >Subject: [PATCH 3/6] ldb_tdb: Ensure the dn in distinguishedName= is valid > before use > >ldb_dn_from_ldb_val() does not validate this untrusted input, so a later >call to ldb_dn_get_casefold() can fail if the input is not valid. > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13374 >--- > lib/ldb/ldb_tdb/ldb_index.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > >diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c >index cf4363f13ac..6aecdca1bfb 100644 >--- a/lib/ldb/ldb_tdb/ldb_index.c >+++ b/lib/ldb/ldb_tdb/ldb_index.c >@@ -959,6 +959,7 @@ static int ltdb_index_dn_leaf(struct ldb_module *module, > return LDB_SUCCESS; > } > if (ldb_attr_dn(tree->u.equality.attr) == 0) { >+ bool valid_dn = false; > struct ldb_dn *dn > = ldb_dn_from_ldb_val(list, > ldb_module_get_ctx(module), >@@ -970,6 +971,14 @@ static int ltdb_index_dn_leaf(struct ldb_module *module, > return LDB_SUCCESS; > } > >+ valid_dn = ldb_dn_validate(dn); >+ if (valid_dn == false) { >+ /* If we can't parse it, no match */ >+ list->dn = NULL; >+ list->count = 0; >+ return LDB_SUCCESS; >+ } >+ > /* > * Re-use the same code we use for a SCOPE_BASE > * search >-- >2.11.0 > > >From e0f954c33135065f0f21139d1b8158a4c75151bd Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 21 May 2018 15:23:53 +1200 >Subject: [PATCH 4/6] ldb_tdb: Check for DN validity in add, rename and search > >This ensures we fail with a good error code before an eventual ldb_dn_get_casefold() which >would otherwise fail. > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13374 >--- > lib/ldb/ldb_tdb/ldb_search.c | 16 ++++++++++++++++ > lib/ldb/ldb_tdb/ldb_tdb.c | 27 ++++++++++++++++++++++++++- > 2 files changed, 42 insertions(+), 1 deletion(-) > >diff --git a/lib/ldb/ldb_tdb/ldb_search.c b/lib/ldb/ldb_tdb/ldb_search.c >index 0af230f219b..b50c9bcba07 100644 >--- a/lib/ldb/ldb_tdb/ldb_search.c >+++ b/lib/ldb/ldb_tdb/ldb_search.c >@@ -292,6 +292,14 @@ int ltdb_search_dn1(struct ldb_module *module, struct ldb_dn *dn, struct ldb_mes > }; > TALLOC_CTX *tdb_key_ctx = NULL; > >+ bool valid_dn = ldb_dn_validate(dn); >+ if (valid_dn == false) { >+ ldb_asprintf_errstring(ldb_module_get_ctx(module), >+ "Invalid Base DN: %s", >+ ldb_dn_get_linearized(dn)); >+ return LDB_ERR_INVALID_DN_SYNTAX; >+ } >+ > if (ltdb->cache->GUID_index_attribute == NULL) { > tdb_key_ctx = talloc_new(msg); > if (!tdb_key_ctx) { >@@ -799,6 +807,14 @@ int ltdb_search(struct ltdb_context *ctx) > ldb_dn_get_linearized(req->op.search.base)); > } > >+ } else if (ldb_dn_validate(req->op.search.base) == false) { >+ >+ /* We don't want invalid base DNs here */ >+ ldb_asprintf_errstring(ldb, >+ "Invalid Base DN: %s", >+ ldb_dn_get_linearized(req->op.search.base)); >+ ret = LDB_ERR_INVALID_DN_SYNTAX; >+ > } else { > /* If we are not checking the base DN life is easy */ > ret = LDB_SUCCESS; >diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c >index a530a454b29..bdf14963e6d 100644 >--- a/lib/ldb/ldb_tdb/ldb_tdb.c >+++ b/lib/ldb/ldb_tdb/ldb_tdb.c >@@ -514,6 +514,16 @@ static int ltdb_add_internal(struct ldb_module *module, > struct ldb_context *ldb = ldb_module_get_ctx(module); > int ret = LDB_SUCCESS; > unsigned int i; >+ bool valid_dn = false; >+ >+ /* Check the new DN is reasonable */ >+ valid_dn = ldb_dn_validate(msg->dn); >+ if (valid_dn == false) { >+ ldb_asprintf_errstring(ldb_module_get_ctx(module), >+ "Invalid DN in ADD: %s", >+ ldb_dn_get_linearized(msg->dn)); >+ return LDB_ERR_INVALID_DN_SYNTAX; >+ } > > for (i=0;i<msg->num_elements;i++) { > struct ldb_message_element *el = &msg->elements[i]; >@@ -1290,6 +1300,7 @@ static int ltdb_rename(struct ltdb_context *ctx) > int ret = LDB_SUCCESS; > TDB_DATA tdb_key, tdb_key_old; > struct ldb_dn *db_dn; >+ bool valid_dn = false; > > ldb_request_set_state(req, LDB_ASYNC_PENDING); > >@@ -1302,10 +1313,24 @@ static int ltdb_rename(struct ltdb_context *ctx) > return LDB_ERR_OPERATIONS_ERROR; > } > >+ /* Check the new DN is reasonable */ >+ valid_dn = ldb_dn_validate(req->op.rename.newdn); >+ if (valid_dn == false) { >+ ldb_asprintf_errstring(ldb_module_get_ctx(module), >+ "Invalid New DN: %s", >+ ldb_dn_get_linearized(req->op.rename.newdn)); >+ return LDB_ERR_INVALID_DN_SYNTAX; >+ } >+ > /* we need to fetch the old record to re-add under the new name */ > ret = ltdb_search_dn1(module, req->op.rename.olddn, msg, > LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC); >- if (ret != LDB_SUCCESS) { >+ if (ret == LDB_ERR_INVALID_DN_SYNTAX) { >+ ldb_asprintf_errstring(ldb_module_get_ctx(module), >+ "Invalid Old DN: %s", >+ ldb_dn_get_linearized(req->op.rename.newdn)); >+ return ret; >+ } else if (ret != LDB_SUCCESS) { > /* not finding the old record is an error */ > return ret; > } >-- >2.11.0 > > >From c1358988d14e4b06fdd4bfe1b5a23edd5b6f7768 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 21 May 2018 15:25:33 +1200 >Subject: [PATCH 5/6] ldb_tdb: Remove pointless check of ldb_dn_is_valid() > >If the DN is not valid the ltdb_search_dn1() will catch it with ldb_dn_validate() which >is the only safe way to check this. ldb_dn_is_valid() does not actually check, but instead >returns only the result of the previous checks, if there was one. > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13374 >--- > lib/ldb/ldb_tdb/ldb_search.c | 8 -------- > 1 file changed, 8 deletions(-) > >diff --git a/lib/ldb/ldb_tdb/ldb_search.c b/lib/ldb/ldb_tdb/ldb_search.c >index b50c9bcba07..abfb6381ec9 100644 >--- a/lib/ldb/ldb_tdb/ldb_search.c >+++ b/lib/ldb/ldb_tdb/ldb_search.c >@@ -767,14 +767,6 @@ int ltdb_search(struct ltdb_context *ctx) > /* We accept subtree searches from a NULL base DN, ie over the whole DB */ > ret = LDB_SUCCESS; > } >- } else if (ldb_dn_is_valid(req->op.search.base) == false) { >- >- /* We don't want invalid base DNs here */ >- ldb_asprintf_errstring(ldb, >- "Invalid Base DN: %s", >- ldb_dn_get_linearized(req->op.search.base)); >- ret = LDB_ERR_INVALID_DN_SYNTAX; >- > } else if (req->op.search.scope == LDB_SCOPE_BASE) { > > /* >-- >2.11.0 > > >From da92c2f9b85ad80ff7be8002a33fc48ecaf77fc8 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 21 May 2018 15:25:58 +1200 >Subject: [PATCH 6/6] ldb: Add tests for search add and rename with a bad dn= > DN > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13374 >--- > lib/ldb/tests/python/api.py | 156 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 156 insertions(+) > >diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py >index 1167517fd5c..ec1951b8304 100755 >--- a/lib/ldb/tests/python/api.py >+++ b/lib/ldb/tests/python/api.py >@@ -401,6 +401,19 @@ class SimpleLdb(LdbBaseTest): > finally: > l.delete(ldb.Dn(l, "dc=bar")) > >+ def test_rename_bad_string_dns(self): >+ l = ldb.Ldb(self.url(), flags=self.flags()) >+ m = ldb.Message() >+ m.dn = ldb.Dn(l, "dc=foo8") >+ m["bla"] = b"bla" >+ m["objectUUID"] = b"0123456789abcdef" >+ self.assertEqual(len(l.search()), 0) >+ l.add(m) >+ self.assertEqual(len(l.search()), 1) >+ self.assertRaises(ldb.LdbError,lambda: l.rename("dcXfoo8", "dc=bar")) >+ self.assertRaises(ldb.LdbError,lambda: l.rename("dc=foo8", "dcXbar")) >+ l.delete(ldb.Dn(l, "dc=foo8")) >+ > def test_empty_dn(self): > l = ldb.Ldb(self.url(), flags=self.flags()) > self.assertEqual(0, len(l.search())) >@@ -1056,6 +1069,110 @@ class SearchTests(LdbBaseTest): > # At some point we should fix this, but it isn't trivial > self.assertEqual(len(res11), 1) > >+ def test_distinguishedName_filter_one(self): >+ """Testing that a distinguishedName= filter succeeds >+ when the scope is SCOPE_ONELEVEL. >+ >+ This should be made more consistent, but for now lock in >+ the behaviour >+ >+ """ >+ >+ res11 = self.l.search(base="DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_ONELEVEL, >+ expression="(distinguishedName=OU=OU1,DC=SAMBA,DC=ORG)") >+ self.assertEqual(len(res11), 1) >+ >+ def test_distinguishedName_filter_subtree(self): >+ """Testing that a distinguishedName= filter succeeds >+ when the scope is SCOPE_SUBTREE""" >+ >+ res11 = self.l.search(base="DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_SUBTREE, >+ expression="(distinguishedName=OU=OU1,DC=SAMBA,DC=ORG)") >+ self.assertEqual(len(res11), 1) >+ >+ def test_distinguishedName_filter_base(self): >+ """Testing that (incorrectly) a distinguishedName= filter works >+ when the scope is SCOPE_BASE""" >+ >+ res11 = self.l.search(base="OU=OU1,DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_BASE, >+ expression="(distinguishedName=OU=OU1,DC=SAMBA,DC=ORG)") >+ >+ # At some point we should fix this, but it isn't trivial >+ self.assertEqual(len(res11), 1) >+ >+ def test_bad_dn_filter_base(self): >+ """Testing that a dn= filter on an invalid DN works >+ when the scope is SCOPE_BASE but >+ returns zero results""" >+ >+ res11 = self.l.search(base="OU=OU1,DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_BASE, >+ expression="(dn=OU=OU1,DC=SAMBA,DCXXXX)") >+ >+ # At some point we should fix this, but it isn't trivial >+ self.assertEqual(len(res11), 0) >+ >+ >+ def test_bad_dn_filter_one(self): >+ """Testing that a dn= filter succeeds but returns zero >+ results when the DN is not valid on a SCOPE_ONELEVEL search >+ >+ """ >+ >+ res11 = self.l.search(base="DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_ONELEVEL, >+ expression="(dn=OU=OU1,DC=SAMBA,DCXXXX)") >+ self.assertEqual(len(res11), 0) >+ >+ def test_bad_dn_filter_subtree(self): >+ """Testing that a dn= filter succeeds but returns zero >+ results when the DN is not valid on a SCOPE_SUBTREE search >+ >+ """ >+ >+ res11 = self.l.search(base="DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_SUBTREE, >+ expression="(dn=OU=OU1,DC=SAMBA,DCXXXX)") >+ self.assertEqual(len(res11), 0) >+ >+ def test_bad_distinguishedName_filter_base(self): >+ """Testing that a distinguishedName= filter on an invalid DN works >+ when the scope is SCOPE_BASE but >+ returns zero results""" >+ >+ res11 = self.l.search(base="OU=OU1,DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_BASE, >+ expression="(distinguishedName=OU=OU1,DC=SAMBA,DCXXXX)") >+ >+ # At some point we should fix this, but it isn't trivial >+ self.assertEqual(len(res11), 0) >+ >+ >+ def test_bad_distinguishedName_filter_one(self): >+ """Testing that a distinguishedName= filter succeeds but returns zero >+ results when the DN is not valid on a SCOPE_ONELEVEL search >+ >+ """ >+ >+ res11 = self.l.search(base="DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_ONELEVEL, >+ expression="(distinguishedName=OU=OU1,DC=SAMBA,DCXXXX)") >+ self.assertEqual(len(res11), 0) >+ >+ def test_bad_distinguishedName_filter_subtree(self): >+ """Testing that a distinguishedName= filter succeeds but returns zero >+ results when the DN is not valid on a SCOPE_SUBTREE search >+ >+ """ >+ >+ res11 = self.l.search(base="DC=SAMBA,DC=ORG", >+ scope=ldb.SCOPE_SUBTREE, >+ expression="(distinguishedName=OU=OU1,DC=SAMBA,DCXXXX)") >+ self.assertEqual(len(res11), 0) >+ > > class IndexedSearchTests(SearchTests): > """Test searches using the index, to ensure the index doesn't >@@ -1189,6 +1306,17 @@ class AddModifyTests(LdbBaseTest): > enum = err.args[0] > self.assertEqual(enum, ldb.ERR_ENTRY_ALREADY_EXISTS) > >+ def test_add_bad(self): >+ try: >+ self.l.add({"dn": "BAD,DC=SAMBA,DC=ORG", >+ "name": b"Admins", >+ "x": "z", "y": "a", >+ "objectUUID": b"0123456789abcde1"}) >+ self.fail("Should have failed adding entry with invalid DN") >+ except ldb.LdbError as err: >+ enum = err.args[0] >+ self.assertEqual(enum, ldb.ERR_INVALID_DN_SYNTAX) >+ > def test_add_del_add(self): > self.l.add({"dn": "OU=DUP,DC=SAMBA,DC=ORG", > "name": b"Admins", >@@ -1270,6 +1398,34 @@ class AddModifyTests(LdbBaseTest): > enum = err.args[0] > self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) > >+ def test_move_bad(self): >+ self.l.add({"dn": "OU=DUP2,DC=SAMBA,DC=ORG", >+ "name": b"Admins", >+ "x": "z", "y": "a", >+ "objectUUID": b"0123456789abcde2"}) >+ >+ try: >+ self.l.rename("OUXDUP,DC=SAMBA,DC=ORG", >+ "OU=DUP2,DC=SAMBA,DC=ORG") >+ self.fail("Should have failed on invalid DN") >+ except ldb.LdbError as err: >+ enum = err.args[0] >+ self.assertEqual(enum, ldb.ERR_INVALID_DN_SYNTAX) >+ >+ def test_move_bad2(self): >+ self.l.add({"dn": "OU=DUP2,DC=SAMBA,DC=ORG", >+ "name": b"Admins", >+ "x": "z", "y": "a", >+ "objectUUID": b"0123456789abcde2"}) >+ >+ try: >+ self.l.rename("OU=DUP,DC=SAMBA,DC=ORG", >+ "OUXDUP2,DC=SAMBA,DC=ORG") >+ self.fail("Should have failed on missing") >+ except ldb.LdbError as err: >+ enum = err.args[0] >+ self.assertEqual(enum, ldb.ERR_INVALID_DN_SYNTAX) >+ > def test_move_fail_move_add(self): > self.l.add({"dn": "OU=DUP,DC=SAMBA,DC=ORG", > "name": b"Admins", >-- >2.11.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 13374
:
14113
|
14206
|
14207
|
14208
|
14267
|
14270
|
14271
|
14272
|
14273
|
14274
|
14275
|
14276
|
14278
|
14279
|
14282
|
14283
|
14289
|
14290
|
14295
|
14296
|
14297
|
14316
|
14317
|
14418
|
14419