Attachment 16026 Details for Bug 14402 – patch for 4.11 to be applied after patch in Bug 14364 (CVE-2020-10730) (v2)

[patch] patch for 4.11 to be applied after patch in Bug 14364 (CVE-2020-10730) (v2)

CVE-2020-10760-4.11-02.patch (text/plain), 12.22 KB, created by Andrew Bartlett on 2020-06-10 05:36:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2020-06-10 05:36:27 UTC

Size: 12.22 KB

patch

obsolete

>From 3e007975601970e33adf9c70bd74eea4fb1a8f8e Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 5 Jun 2020 22:14:48 +1200
>Subject: [PATCH 1/2] CVE-2020-10760 dsdb: Ensure a proper talloc tree for
> saved controls
>
>Otherwise a paged search on the GC port will fail as the ->data was
>not kept around for the second page of searches.
>
>An example command to produce this is
> bin/ldbsearch --paged -H ldap://$SERVER:3268 -U$USERNAME%$PASSWORD
>
>This shows up later in the partition module as:
>
>ERROR: AddressSanitizer: heap-use-after-free on address 0x60b00151ef20 at pc 0x7fec3f801aac bp 0x7ffe8472c270 sp 0x7ffe8472c260
>READ of size 4 at 0x60b00151ef20 thread T0 (ldap(0))
>    #0 0x7fec3f801aab in talloc_chunk_from_ptr ../../lib/talloc/talloc.c:526
>    #1 0x7fec3f801aab in __talloc_get_name ../../lib/talloc/talloc.c:1559
>    #2 0x7fec3f801aab in talloc_check_name ../../lib/talloc/talloc.c:1582
>    #3 0x7fec1b86b2e1 in partition_search ../../source4/dsdb/samdb/ldb_modules/partition.c:780
>
>or
>
>smb_panic_default: PANIC (pid 13287): Bad talloc magic value - unknown value
>(from source4/dsdb/samdb/ldb_modules/partition.c:780)
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14402
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/paged_results.c  | 8 ++++++++
> source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 7 +++++++
> 2 files changed, 15 insertions(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/paged_results.c b/source4/dsdb/samdb/ldb_modules/paged_results.c
>index 735883e8802..3eea3236e7d 100644
>--- a/source4/dsdb/samdb/ldb_modules/paged_results.c
>+++ b/source4/dsdb/samdb/ldb_modules/paged_results.c
>@@ -523,6 +523,14 @@ paged_results_copy_down_controls(TALLOC_CTX *mem_ctx,
> 			continue;
> 		}
> 		new_controls[j] = talloc_steal(new_controls, control);
>+
>+		/*
>+		 * Sadly the caller is not obliged to make this a
>+		 * proper talloc tree, so we do so here.
>+		 */
>+		if (control->data) {
>+			talloc_steal(control, control->data);
>+		}
> 		j++;
> 	}
> 	new_controls[j] = NULL;
>diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>index b103bda5f52..d6d6039e849 100644
>--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>@@ -746,6 +746,13 @@ vlv_copy_down_controls(TALLOC_CTX *mem_ctx, struct ldb_control **controls)
> 			continue;
> 		}
> 		new_controls[j] = talloc_steal(new_controls, control);
>+		/*
>+		 * Sadly the caller is not obliged to make this a
>+		 * proper talloc tree, so we do so here.
>+		 */
>+		if (control->data) {
>+			talloc_steal(control, control->data);
>+		}
> 		j++;
> 	}
> 	new_controls[j] = NULL;
>-- 
>2.17.1
>
>
>From b29509f18c5b16a455cabae4b26b8d490cfd5725 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 8 Jun 2020 16:32:14 +1200
>Subject: [PATCH 2/2] CVE-2020-10760 dsdb: Add tests for paged_results and VLV
> over the Global Catalog port
>
>This should avoid a regression.
>
>(backported from master patch)
>[abartlet@samba.org: sort=True parameter on test_paged_delete_during_search
> is not in 4.11]
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> selftest/knownfail.d/vlv         |   2 +-
> source4/dsdb/tests/python/vlv.py | 171 +++++++++++++++++++------------
> 2 files changed, 107 insertions(+), 66 deletions(-)
>
>diff --git a/selftest/knownfail.d/vlv b/selftest/knownfail.d/vlv
>index f187a2ed55e..7ae02baf17b 100644
>--- a/selftest/knownfail.d/vlv
>+++ b/selftest/knownfail.d/vlv
>@@ -1,2 +1,2 @@
> samba4.ldap.vlv.python.*__main__.VLVTests.test_vlv_change_search_expr
>-samba4.ldap.vlv.python.*__main__.PagedResultsTests.test_paged_cant_change_controls_data
>+samba4.ldap.vlv.python.*__main__.PagedResultsTestsRW.test_paged_cant_change_controls_data
>diff --git a/source4/dsdb/tests/python/vlv.py b/source4/dsdb/tests/python/vlv.py
>index f3c603e3a39..ba03b425a5b 100644
>--- a/source4/dsdb/tests/python/vlv.py
>+++ b/source4/dsdb/tests/python/vlv.py
>@@ -152,7 +152,7 @@ class TestsWithUserOU(samba.tests.TestCase):
>         super(TestsWithUserOU, self).setUp()
>         self.ldb = SamDB(host, credentials=creds,
>                          session_info=system_session(lp), lp=lp)
>-
>+        self.ldb_ro = self.ldb
>         self.base_dn = self.ldb.domain_dn()
>         self.tree_dn = "ou=vlvtesttree,%s" % self.base_dn
>         self.ou = "ou=vlvou,%s" % self.tree_dn
>@@ -199,8 +199,60 @@ class TestsWithUserOU(samba.tests.TestCase):
>             self.ldb.delete(self.tree_dn, ['tree_delete:1'])
> 
> 
>-class VLVTests(TestsWithUserOU):
>+class VLVTestsBase(TestsWithUserOU):
>+
>+    # Run a vlv search and return important fields of the response control
>+    def vlv_search(self, attr, expr, cookie="", after_count=0, offset=1):
>+        sort_ctrl = "server_sort:1:0:%s" % attr
>+        ctrl = "vlv:1:0:%d:%d:0" % (after_count, offset)
>+        if cookie:
>+            ctrl += ":" + cookie
>+
>+        res = self.ldb_ro.search(self.ou,
>+                              expression=expr,
>+                              scope=ldb.SCOPE_ONELEVEL,
>+                              attrs=[attr],
>+                              controls=[ctrl, sort_ctrl])
>+        results = [str(x[attr][0]) for x in res]
>+
>+        ctrls = [str(c) for c in res.controls if
>+                 str(c).startswith('vlv')]
>+        self.assertEqual(len(ctrls), 1)
>+
>+        spl = ctrls[0].rsplit(':')
>+        cookie = ""
>+        if len(spl) == 6:
>+            cookie = spl[-1]
>+
>+        return results, cookie
>+
>+
>+class VLVTestsRO(VLVTestsBase):
>+    def test_vlv_simple_double_run(self):
>+        """Do the simplest possible VLV query to confirm if VLV
>+           works at all.  Useful for showing VLV as a whole works
>+           on Global Catalog (for example)"""
>+        attr = 'roomNumber'
>+        expr = "(objectclass=user)"
> 
>+        # Start new search
>+        full_results, cookie = self.vlv_search(attr, expr,
>+                                               after_count=len(self.users))
>+
>+        results, cookie = self.vlv_search(attr, expr, cookie=cookie,
>+                                          after_count=len(self.users))
>+        expected_results = full_results
>+        self.assertEqual(results, expected_results)
>+
>+
>+class VLVTestsGC(VLVTestsRO):
>+    def setUp(self):
>+        super(VLVTestsRO, self).setUp()
>+        self.ldb_ro = SamDB(host + ":3268", credentials=creds,
>+                            session_info=system_session(lp), lp=lp)
>+
>+
>+class VLVTests(VLVTestsBase):
>     def get_full_list(self, attr, include_cn=False):
>         """Fetch the whole list sorted on the attribute, using the VLV.
>         This way you get a VLV cookie."""
>@@ -1081,31 +1133,6 @@ class VLVTests(TestsWithUserOU):
>                        controls=[sort_control,
>                                  "vlv:0:1:1:1:0:%s" % vlv_cookies[-1]])
> 
>-    # Run a vlv search and return important fields of the response control
>-    def vlv_search(self, attr, expr, cookie="", after_count=0, offset=1):
>-        sort_ctrl = "server_sort:1:0:%s" % attr
>-        ctrl = "vlv:1:0:%d:%d:0" % (after_count, offset)
>-        if cookie:
>-            ctrl += ":" + cookie
>-
>-        res = self.ldb.search(self.ou,
>-                              expression=expr,
>-                              scope=ldb.SCOPE_ONELEVEL,
>-                              attrs=[attr],
>-                              controls=[ctrl, sort_ctrl])
>-        results = [str(x[attr][0]) for x in res]
>-
>-        ctrls = [str(c) for c in res.controls if
>-                 str(c).startswith('vlv')]
>-        self.assertEqual(len(ctrls), 1)
>-
>-        spl = ctrls[0].rsplit(':')
>-        cookie = ""
>-        if len(spl) == 6:
>-            cookie = spl[-1]
>-
>-        return results, cookie
>-
>     def test_vlv_modify_during_view(self):
>         attr = 'roomNumber'
>         expr = "(objectclass=user)"
>@@ -1218,11 +1245,11 @@ class PagedResultsTests(TestsWithUserOU):
>         if subtree:
>             scope = ldb.SCOPE_SUBTREE
> 
>-        res = self.ldb.search(ou,
>-                              expression=expr,
>-                              scope=scope,
>-                              controls=controls,
>-                              **kwargs)
>+        res = self.ldb_ro.search(ou,
>+                                 expression=expr,
>+                                 scope=scope,
>+                                 controls=controls,
>+                                 **kwargs)
>         results = [str(r['cn'][0]) for r in res]
> 
>         ctrls = [str(c) for c in res.controls if
>@@ -1235,6 +1262,53 @@ class PagedResultsTests(TestsWithUserOU):
>             cookie = spl[-1]
>         return results, cookie
> 
>+
>+class PagedResultsTestsRO(PagedResultsTests):
>+
>+    def test_paged_search_lockstep(self):
>+        expr = "(objectClass=*)"
>+        ps = 3
>+
>+        all_results, _ = self.paged_search(expr, page_size=len(self.users)+1)
>+
>+        # Run two different but overlapping paged searches simultaneously.
>+        set_1_index = int((len(all_results))//3)
>+        set_2_index = int((2*len(all_results))//3)
>+        set_1 = all_results[set_1_index:]
>+        set_2 = all_results[:set_2_index+1]
>+        set_1_expr = "(cn>=%s)" % (all_results[set_1_index])
>+        set_2_expr = "(cn<=%s)" % (all_results[set_2_index])
>+
>+        results, cookie1 = self.paged_search(set_1_expr, page_size=ps)
>+        self.assertEqual(results, set_1[:ps])
>+        results, cookie2 = self.paged_search(set_2_expr, page_size=ps)
>+        self.assertEqual(results, set_2[:ps])
>+
>+        results, cookie1 = self.paged_search(set_1_expr, cookie=cookie1,
>+                                             page_size=ps)
>+        self.assertEqual(results, set_1[ps:ps*2])
>+        results, cookie2 = self.paged_search(set_2_expr, cookie=cookie2,
>+                                             page_size=ps)
>+        self.assertEqual(results, set_2[ps:ps*2])
>+
>+        results, _ = self.paged_search(set_1_expr, cookie=cookie1,
>+                                       page_size=len(self.users))
>+        self.assertEqual(results, set_1[ps*2:])
>+        results, _ = self.paged_search(set_2_expr, cookie=cookie2,
>+                                       page_size=len(self.users))
>+        self.assertEqual(results, set_2[ps*2:])
>+
>+
>+class PagedResultsTestsGC(PagedResultsTestsRO):
>+
>+    def setUp(self):
>+        super(PagedResultsTestsRO, self).setUp()
>+        self.ldb_ro = SamDB(host + ":3268", credentials=creds,
>+                            session_info=system_session(lp), lp=lp)
>+
>+
>+class PagedResultsTestsRW(PagedResultsTests):
>+
>     def test_paged_delete_during_search(self):
>         expr = "(objectClass=*)"
> 
>@@ -1611,39 +1685,6 @@ class PagedResultsTests(TestsWithUserOU):
>                                      cookie, attrs=changed_attrs,
>                                      extra_ctrls=[])
> 
>-    def test_paged_search_lockstep(self):
>-        expr = "(objectClass=*)"
>-        ps = 3
>-
>-        all_results, _ = self.paged_search(expr, page_size=len(self.users)+1)
>-
>-        # Run two different but overlapping paged searches simultaneously.
>-        set_1_index = int((len(all_results))//3)
>-        set_2_index = int((2*len(all_results))//3)
>-        set_1 = all_results[set_1_index:]
>-        set_2 = all_results[:set_2_index+1]
>-        set_1_expr = "(cn>=%s)" % (all_results[set_1_index])
>-        set_2_expr = "(cn<=%s)" % (all_results[set_2_index])
>-
>-        results, cookie1 = self.paged_search(set_1_expr, page_size=ps)
>-        self.assertEqual(results, set_1[:ps])
>-        results, cookie2 = self.paged_search(set_2_expr, page_size=ps)
>-        self.assertEqual(results, set_2[:ps])
>-
>-        results, cookie1 = self.paged_search(set_1_expr, cookie=cookie1,
>-                                             page_size=ps)
>-        self.assertEqual(results, set_1[ps:ps*2])
>-        results, cookie2 = self.paged_search(set_2_expr, cookie=cookie2,
>-                                             page_size=ps)
>-        self.assertEqual(results, set_2[ps:ps*2])
>-
>-        results, _ = self.paged_search(set_1_expr, cookie=cookie1,
>-                                       page_size=len(self.users))
>-        self.assertEqual(results, set_1[ps*2:])
>-        results, _ = self.paged_search(set_2_expr, cookie=cookie2,
>-                                       page_size=len(self.users))
>-        self.assertEqual(results, set_2[ps*2:])
>-
>     def test_vlv_paged(self):
>         """Testing behaviour with VLV and paged_results set.
> 
>-- 
>2.17.1
>

Flags:

abartlet: review? (gary)
gary: review+
abartlet: ci-passed+

Actions: View

Attachments on bug 14402: 16021 | 16022 | 16023 | 16024 | 16025 | 16026 | 16027 | 16028 | 16029 | 16058 | 16066