main/samba: fix CVE-2019-14902, CVE-2019-14907

fixes #11155

2 files changed, 1668 insertions, 1 deletions

diff --git a/main/samba/APKBUILD b/main/samba/APKBUILD
index 6543808e76..50d1748996 100644
--- a/main/samba/APKBUILD
+++ b/main/samba/APKBUILD
@@ -1,7 +1,7 @@
 # Maintainer: Natanael Copa <ncopa@alpinelinux.org>
 pkgname=samba
 pkgver=4.8.12
-pkgrel=1
+pkgrel=2
 pkgdesc="Tools to access a server's filespace and printers via SMB"
 url="https://www.samba.org/"
 arch="all"
@@ -51,6 +51,7 @@ source="https://us1.samba.org/samba/ftp/stable/$pkgname-$pkgver.tar.gz
 	netdb-defines.patch
 	netapp.patch
 	samba-4.9.14-security-2019-10-29.patch
+	samba-4.9.17-security-2020-01-21.patch
 	$pkgname.initd
 	$pkgname.confd
 	$pkgname.logrotate
@@ -59,6 +60,9 @@ pkggroups="winbind"
 builddir="$srcdir/$pkgname-$pkgver"
 
 # secfixes:
+#   4.8.12-r2:
+#     - CVE-2019-14902
+#     - CVE-2019-14907
 #   4.8.12-r1:
 #     - CVE-2019-10218
 #     - CVE-2019-14833
@@ -539,6 +543,7 @@ a99e771f28d787dc22e832b97aa48a1c5e13ddc0c030c501a3c12819ff6e62800ef084b62930abe8
 1854577d0e4457e27da367a6c7ec0fb5cfd63cefea0a39181c9d6e78cf8d3eb50878cdddeea3daeec955d00263151c2f86ea754ff4276ef98bc52c0276d9ffe8  netdb-defines.patch
 202667cb0383414d9289cd67574f5e1140c9a0ff63bb82a746a59b2397a00db15654bfb30cb5ec1cd68a097899be0f849d9aab4c0d210152386c9e66c640f0c0  netapp.patch
 8386db1209721fabb6acf52e498082ac3e70cd3a4454c54416b02aaa67b2906212383da7ddc06f77ca29cfbb9033407b1e958bcd9c7cdf369fe501f310a0f973  samba-4.9.14-security-2019-10-29.patch
+b00163634fb262777cc8992192150beb5dc2dc45ace823557f1a35fe2448ab3559b7503db96b07c6a9382ddb62a3bd6f4e68e1849f64ec472dbea8abc6b54572  samba-4.9.17-security-2020-01-21.patch
 8ffd93a2f8f1461d3eabf3cbc67f04f13d18d02b969e110b1cf5f23845264d774a7a79658cdf389bc594780c633266ff29aacc81dae627dcb3efe63364b027bd  samba.initd
 4faf581ecef3ec38319e3c4ab6d3995c51fd7ba83180dc5553a2ff4dfb92efadb43030c543292130c4ed0c281dc0972c6973d52d48062c5edb39bb1c4bbb6dd6  samba.confd
 3458a4e1f8a8b44c966afb339b2dca51615be049f594c14911fc4d8203623deee416b6fe881436e246fc7d49c97a2b3bf9c5f33ba774302b24190a1103d6b67d  samba.logrotate"
diff --git a/main/samba/samba-4.9.17-security-2020-01-21.patch b/main/samba/samba-4.9.17-security-2020-01-21.patch
new file mode 100644
index 0000000000..4847a8660b
--- /dev/null
+++ b/main/samba/samba-4.9.17-security-2020-01-21.patch
@@ -0,0 +1,1662 @@
+From 77d55b64af6acd38a08096b89ee051bc4ce72f43 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Thu, 28 Nov 2019 17:16:16 +1300
+Subject: [PATCH 01/13] CVE-2019-14902 selftest: Add test for replication of
+ inherited security descriptors
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ selftest/knownfail.d/repl_secdesc          |   2 +
+ source4/selftest/tests.py                  |   5 +
+ source4/torture/drs/python/repl_secdesc.py | 258 +++++++++++++++++++++
+ 3 files changed, 265 insertions(+)
+ create mode 100644 selftest/knownfail.d/repl_secdesc
+ create mode 100644 source4/torture/drs/python/repl_secdesc.py
+
+diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc
+new file mode 100644
+index 00000000000..2aa24c61375
+--- /dev/null
++++ b/selftest/knownfail.d/repl_secdesc
+@@ -0,0 +1,2 @@
++^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_object_in_conflict
++^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inherit_existing_object
+diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
+index 2ec0bee923b..7244535791d 100755
+--- a/source4/selftest/tests.py
++++ b/source4/selftest/tests.py
+@@ -1004,6 +1004,11 @@ for env in ['vampire_dc', 'promoted_dc']:
+                            extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
+                            environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
+                            extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
++    planoldpythontestsuite(env, "repl_secdesc",
++                           name="samba4.drs.repl_secdesc.python(%s)" % env,
++                           extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
++                           environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'},
++                           extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
+     planoldpythontestsuite(env, "repl_move",
+                            extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
+                            name="samba4.drs.repl_move.python(%s)" % env,
+diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py
+new file mode 100644
+index 00000000000..4ed449a8a18
+--- /dev/null
++++ b/source4/torture/drs/python/repl_secdesc.py
+@@ -0,0 +1,258 @@
++#!/usr/bin/env python3
++# -*- coding: utf-8 -*-
++#
++# Unix SMB/CIFS implementation.
++# Copyright (C) Catalyst.Net Ltd. 2017
++# Copyright (C) Andrew Bartlett <abartlet@samba.org> 2019
++#
++# This program is free software; you can redistribute it and/or modify
++# it under the terms of the GNU General Public License as published by
++# the Free Software Foundation; either version 3 of the License, or
++# (at your option) any later version.
++#
++# This program is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program.  If not, see <http://www.gnu.org/licenses/>.
++#
++import drs_base
++import ldb
++import samba
++from samba import sd_utils
++from ldb import LdbError
++
++class ReplAclTestCase(drs_base.DrsBaseTestCase):
++
++    def setUp(self):
++        super(ReplAclTestCase, self).setUp()
++        self.sd_utils_dc1 = sd_utils.SDUtils(self.ldb_dc1)
++        self.sd_utils_dc2 = sd_utils.SDUtils(self.ldb_dc2)
++
++        self.ou = samba.tests.create_test_ou(self.ldb_dc1,
++                                             "test_acl_inherit")
++
++        # disable replication for the tests so we can control at what point
++        # the DCs try to replicate
++        self._disable_all_repl(self.dnsname_dc1)
++        self._disable_all_repl(self.dnsname_dc2)
++
++        # make sure DCs are synchronized before the test
++        self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1, forced=True)
++        self._net_drs_replicate(DC=self.dnsname_dc1, fromDC=self.dnsname_dc2, forced=True)
++
++    def tearDown(self):
++        self.ldb_dc1.delete(self.ou, ["tree_delete:1"])
++
++        # re-enable replication
++        self._enable_all_repl(self.dnsname_dc1)
++        self._enable_all_repl(self.dnsname_dc2)
++
++        super(ReplAclTestCase, self).tearDown()
++
++    def test_acl_inheirt_new_object_1_pass(self):
++        # Set the inherited ACL on the parent OU
++        mod =  "(A;CIOI;GA;;;SY)"
++        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++
++        # Make a new object
++        dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
++        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"})
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm inherited ACLs are identical
++
++        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
++                          self.sd_utils_dc2.get_sd_as_sddl(dn))
++
++    def test_acl_inheirt_new_object(self):
++        # Set the inherited ACL on the parent OU
++        mod =  "(A;CIOI;GA;;;SY)"
++        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++
++        # Replicate to DC2
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Make a new object
++        dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
++        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"})
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm inherited ACLs are identical
++
++        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
++                          self.sd_utils_dc2.get_sd_as_sddl(dn))
++
++    def test_acl_inherit_existing_object(self):
++        # Make a new object
++        dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
++        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"})
++
++        try:
++            self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
++                                base=dn,
++                                attrs=[])
++            self.fail()
++        except LdbError as err:
++            enum = err.args[0]
++            self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT)
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm it is now replicated
++        self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
++                            base=dn,
++                            attrs=[])
++
++        # Set the inherited ACL on the parent OU
++        mod =  "(A;CIOI;GA;;;SY)"
++        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++
++        # Replicate to DC2
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm inherited ACLs are identical
++
++        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
++                          self.sd_utils_dc2.get_sd_as_sddl(dn))
++
++    def test_acl_inheirt_existing_object_1_pass(self):
++        # Make a new object
++        dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
++        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"})
++
++        try:
++            self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
++                                base=dn,
++                                attrs=[])
++            self.fail()
++        except LdbError as err:
++            enum = err.args[0]
++            self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT)
++
++        # Set the inherited ACL on the parent OU
++        mod =  "(A;CIOI;GA;;;SY)"
++        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++
++        # Replicate to DC2
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm inherited ACLs are identical
++
++        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
++                          self.sd_utils_dc2.get_sd_as_sddl(dn))
++
++    def test_acl_inheirt_renamed_object(self):
++        # Make a new object
++        new_ou = samba.tests.create_test_ou(self.ldb_dc1,
++                                            "acl_test_l2")
++
++        sub_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
++
++        try:
++            self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
++                                base=new_ou,
++                                attrs=[])
++            self.fail()
++        except LdbError as err:
++            enum = err.args[0]
++            self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT)
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm it is now replicated
++        self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
++                            base=new_ou,
++                            attrs=[])
++
++        # Set the inherited ACL on the parent OU on DC1
++        mod =  "(A;CIOI;GA;;;SY)"
++        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++
++        # Replicate to DC2
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Rename to under self.ou
++
++        self.ldb_dc1.rename(new_ou, sub_ou_dn)
++
++        # Replicate to DC2
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm inherited ACLs are identical
++        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn),
++                          self.sd_utils_dc2.get_sd_as_sddl(sub_ou_dn))
++
++
++    def test_acl_inheirt_renamed_object_in_conflict(self):
++        # Make a new object to be renamed under self.ou
++        new_ou = samba.tests.create_test_ou(self.ldb_dc1,
++                                            "acl_test_l2")
++
++        # Make a new OU under self.ou (on DC2)
++        sub_ou_dn = ldb.Dn(self.ldb_dc2, "OU=l2,%s" % self.ou)
++        self.ldb_dc2.add({"dn": sub_ou_dn,
++                          "objectclass": "organizationalUnit"})
++
++        # Set the inherited ACL on the parent OU
++        mod =  "(A;CIOI;GA;;;SY)"
++        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++
++        # Replicate to DC2
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Rename to under self.ou
++        self.ldb_dc1.rename(new_ou, sub_ou_dn)
++
++        # Replicate to DC2 (will cause a conflict, DC1 to win, version
++        # is higher since named twice)
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        children = self.ldb_dc2.search(scope=ldb.SCOPE_ONELEVEL,
++                                       base=self.ou,
++                                       attrs=[])
++        for child in children:
++            self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn),
++                              self.sd_utils_dc2.get_sd_as_sddl(child.dn))
++
++        # Replicate back
++        self._net_drs_replicate(DC=self.dnsname_dc1,
++                                fromDC=self.dnsname_dc2,
++                                forced=True)
++
++        for child in children:
++            self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(child.dn),
++                              self.sd_utils_dc2.get_sd_as_sddl(child.dn))
+-- 
+2.17.1
+
+
+From c5a005a45389c8d8fc0eae7137eab1904ea92d42 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Tue, 10 Dec 2019 15:16:24 +1300
+Subject: [PATCH 02/13] CVE-2019-14902 selftest: Add test for a special case
+ around replicated renames
+
+It appears Samba is currently string-name based in the ACL inheritence code.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ selftest/knownfail.d/repl_secdesc          |  1 +
+ source4/torture/drs/python/repl_secdesc.py | 69 ++++++++++++++++++++++
+ 2 files changed, 70 insertions(+)
+
+diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc
+index 2aa24c61375..7d554ff237a 100644
+--- a/selftest/knownfail.d/repl_secdesc
++++ b/selftest/knownfail.d/repl_secdesc
+@@ -1,2 +1,3 @@
+ ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_object_in_conflict
+ ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inherit_existing_object
++^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_child_object
+diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py
+index 4ed449a8a18..58861af3bac 100644
+--- a/source4/torture/drs/python/repl_secdesc.py
++++ b/source4/torture/drs/python/repl_secdesc.py
+@@ -211,6 +211,75 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                           self.sd_utils_dc2.get_sd_as_sddl(sub_ou_dn))
+ 
+ 
++    def test_acl_inheirt_renamed_child_object(self):
++        # Make a new OU
++        new_ou = samba.tests.create_test_ou(self.ldb_dc1,
++                                            "acl_test_l2")
++
++        # Here is where the new OU will end up at the end.
++        sub2_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
++
++        sub3_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l3,%s" % new_ou)
++        sub3_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l3,%s" % sub2_ou_dn_final)
++
++        self.ldb_dc1.add({"dn": sub3_ou_dn,
++                          "objectclass": "organizationalUnit"})
++
++        sub4_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l4,%s" % sub3_ou_dn)
++        sub4_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l4,%s" % sub3_ou_dn_final)
++
++        self.ldb_dc1.add({"dn": sub4_ou_dn,
++                          "objectclass": "organizationalUnit"})
++
++        try:
++            self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
++                                base=new_ou,
++                                attrs=[])
++            self.fail()
++        except LdbError as err:
++            enum = err.args[0]
++            self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT)
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm it is now replicated
++        self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
++                            base=new_ou,
++                            attrs=[])
++
++        #
++        # Given a tree new_ou -> l3 -> l4
++        #
++
++        # Set the inherited ACL on the grandchild OU (l3) on DC1
++        mod =  "(A;CIOI;GA;;;SY)"
++        self.sd_utils_dc1.dacl_add_ace(sub3_ou_dn, mod)
++
++        # Rename new_ou (l2) to under self.ou (this must happen second).  If the
++        # inheritence between l3 and l4 is name-based, this could
++        # break.
++
++        # The tree is now self.ou -> l2 -> l3 -> l4
++
++        self.ldb_dc1.rename(new_ou, sub2_ou_dn_final)
++
++        # Replicate to DC2
++
++        self._net_drs_replicate(DC=self.dnsname_dc2,
++                                fromDC=self.dnsname_dc1,
++                                forced=True)
++
++        # Confirm set ACLs (on l3 ) are identical.
++        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn_final),
++                          self.sd_utils_dc2.get_sd_as_sddl(sub3_ou_dn_final))
++
++        # Confirm inherited ACLs (from l3 to l4) are identical.
++        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub4_ou_dn_final),
++                          self.sd_utils_dc2.get_sd_as_sddl(sub4_ou_dn_final))
++
++
+     def test_acl_inheirt_renamed_object_in_conflict(self):
+         # Make a new object to be renamed under self.ou
+         new_ou = samba.tests.create_test_ou(self.ldb_dc1,
+-- 
+2.17.1
+
+
+From 4afff32debe5ea4bf1219f42c3042eb65c3e1d6b Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Mon, 16 Dec 2019 11:29:27 +1300
+Subject: [PATCH 03/13] selftest: Add test to confirm ACL inheritence really
+ happens
+
+While we have a seperate test (sec_descriptor.py) that confirms inheritance in
+general we want to lock in these specific patterns as this test covers
+rename.
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ source4/torture/drs/python/repl_secdesc.py | 115 +++++++++++++++++----
+ 1 file changed, 94 insertions(+), 21 deletions(-)
+
+diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py
+index 58861af3bac..58212907e23 100644
+--- a/source4/torture/drs/python/repl_secdesc.py
++++ b/source4/torture/drs/python/repl_secdesc.py
+@@ -28,6 +28,10 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+ 
+     def setUp(self):
+         super(ReplAclTestCase, self).setUp()
++        self.mod = "(A;CIOI;GA;;;SY)"
++        self.mod_becomes = "(A;OICIIO;GA;;;SY)"
++        self.mod_inherits_as = "(A;OICIIOID;GA;;;SY)"
++
+         self.sd_utils_dc1 = sd_utils.SDUtils(self.ldb_dc1)
+         self.sd_utils_dc2 = sd_utils.SDUtils(self.ldb_dc2)
+ 
+@@ -54,8 +58,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+ 
+     def test_acl_inheirt_new_object_1_pass(self):
+         # Set the inherited ACL on the parent OU
+-        mod =  "(A;CIOI;GA;;;SY)"
+-        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++        self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod)
++
++        # Assert ACL set stuck as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc1.get_sd_as_sddl(self.ou))
+ 
+         # Make a new object
+         dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
+@@ -65,15 +72,24 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                                 fromDC=self.dnsname_dc1,
+                                 forced=True)
+ 
+-        # Confirm inherited ACLs are identical
++        # Assert ACL replicated as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc2.get_sd_as_sddl(self.ou))
+ 
++        # Confirm inherited ACLs are identical and were inherited
++
++        self.assertIn(self.mod_inherits_as,
++                      self.sd_utils_dc1.get_sd_as_sddl(dn))
+         self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
+                           self.sd_utils_dc2.get_sd_as_sddl(dn))
+ 
+     def test_acl_inheirt_new_object(self):
+         # Set the inherited ACL on the parent OU
+-        mod =  "(A;CIOI;GA;;;SY)"
+-        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++        self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod)
++
++        # Assert ACL set stuck as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc1.get_sd_as_sddl(self.ou))
+ 
+         # Replicate to DC2
+ 
+@@ -89,8 +105,14 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                                 fromDC=self.dnsname_dc1,
+                                 forced=True)
+ 
+-        # Confirm inherited ACLs are identical
++        # Assert ACL replicated as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc2.get_sd_as_sddl(self.ou))
+ 
++        # Confirm inherited ACLs are identical and were inheritied
++
++        self.assertIn(self.mod_inherits_as,
++                      self.sd_utils_dc1.get_sd_as_sddl(dn))
+         self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
+                           self.sd_utils_dc2.get_sd_as_sddl(dn))
+ 
+@@ -118,8 +140,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                             attrs=[])
+ 
+         # Set the inherited ACL on the parent OU
+-        mod =  "(A;CIOI;GA;;;SY)"
+-        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++        self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod)
++
++        # Assert ACL set stuck as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc1.get_sd_as_sddl(self.ou))
+ 
+         # Replicate to DC2
+ 
+@@ -127,8 +152,14 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                                 fromDC=self.dnsname_dc1,
+                                 forced=True)
+ 
+-        # Confirm inherited ACLs are identical
++        # Confirm inherited ACLs are identical and were inherited
+ 
++        # Assert ACL replicated as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc2.get_sd_as_sddl(self.ou))
++
++        self.assertIn(self.mod_inherits_as,
++                      self.sd_utils_dc1.get_sd_as_sddl(dn))
+         self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
+                           self.sd_utils_dc2.get_sd_as_sddl(dn))
+ 
+@@ -147,8 +178,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+             self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT)
+ 
+         # Set the inherited ACL on the parent OU
+-        mod =  "(A;CIOI;GA;;;SY)"
+-        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++        self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod)
++
++        # Assert ACL set as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc1.get_sd_as_sddl(self.ou))
+ 
+         # Replicate to DC2
+ 
+@@ -156,8 +190,14 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                                 fromDC=self.dnsname_dc1,
+                                 forced=True)
+ 
+-        # Confirm inherited ACLs are identical
++        # Assert ACL replicated as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc2.get_sd_as_sddl(self.ou))
+ 
++        # Confirm inherited ACLs are identical and were inherited
++
++        self.assertIn(self.mod_inherits_as,
++                      self.sd_utils_dc1.get_sd_as_sddl(dn))
+         self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
+                           self.sd_utils_dc2.get_sd_as_sddl(dn))
+ 
+@@ -187,8 +227,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                             attrs=[])
+ 
+         # Set the inherited ACL on the parent OU on DC1
+-        mod =  "(A;CIOI;GA;;;SY)"
+-        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++        self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod)
++
++        # Assert ACL set as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc1.get_sd_as_sddl(self.ou))
+ 
+         # Replicate to DC2
+ 
+@@ -196,6 +239,10 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                                 fromDC=self.dnsname_dc1,
+                                 forced=True)
+ 
++        # Assert ACL replicated as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc2.get_sd_as_sddl(self.ou))
++
+         # Rename to under self.ou
+ 
+         self.ldb_dc1.rename(new_ou, sub_ou_dn)
+@@ -206,7 +253,9 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                                 fromDC=self.dnsname_dc1,
+                                 forced=True)
+ 
+-        # Confirm inherited ACLs are identical
++        # Confirm inherited ACLs are identical and were inherited
++        self.assertIn(self.mod_inherits_as,
++                      self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn))
+         self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn),
+                           self.sd_utils_dc2.get_sd_as_sddl(sub_ou_dn))
+ 
+@@ -254,8 +303,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+         #
+ 
+         # Set the inherited ACL on the grandchild OU (l3) on DC1
+-        mod =  "(A;CIOI;GA;;;SY)"
+-        self.sd_utils_dc1.dacl_add_ace(sub3_ou_dn, mod)
++        self.sd_utils_dc1.dacl_add_ace(sub3_ou_dn, self.mod)
++
++        # Assert ACL set stuck as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn))
+ 
+         # Rename new_ou (l2) to under self.ou (this must happen second).  If the
+         # inheritence between l3 and l4 is name-based, this could
+@@ -265,17 +317,26 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+ 
+         self.ldb_dc1.rename(new_ou, sub2_ou_dn_final)
+ 
++        # Assert ACL set remained as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn_final))
++
+         # Replicate to DC2
+ 
+         self._net_drs_replicate(DC=self.dnsname_dc2,
+                                 fromDC=self.dnsname_dc1,
+                                 forced=True)
+ 
+-        # Confirm set ACLs (on l3 ) are identical.
++        # Confirm set ACLs (on l3 ) are identical and were inherited
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc2.get_sd_as_sddl(sub3_ou_dn_final))
+         self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn_final),
+                           self.sd_utils_dc2.get_sd_as_sddl(sub3_ou_dn_final))
+ 
+-        # Confirm inherited ACLs (from l3 to l4) are identical.
++        # Confirm inherited ACLs (from l3 to l4) are identical
++        # and where inherited
++        self.assertIn(self.mod_inherits_as,
++                      self.sd_utils_dc1.get_sd_as_sddl(sub4_ou_dn_final))
+         self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub4_ou_dn_final),
+                           self.sd_utils_dc2.get_sd_as_sddl(sub4_ou_dn_final))
+ 
+@@ -291,8 +352,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                           "objectclass": "organizationalUnit"})
+ 
+         # Set the inherited ACL on the parent OU
+-        mod =  "(A;CIOI;GA;;;SY)"
+-        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
++        self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod)
++
++        # Assert ACL set stuck as expected
++        self.assertIn(self.mod_becomes,
++                      self.sd_utils_dc1.get_sd_as_sddl(self.ou))
+ 
+         # Replicate to DC2
+ 
+@@ -302,6 +366,8 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+ 
+         # Rename to under self.ou
+         self.ldb_dc1.rename(new_ou, sub_ou_dn)
++        self.assertIn(self.mod_inherits_as,
++                      self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn))
+ 
+         # Replicate to DC2 (will cause a conflict, DC1 to win, version
+         # is higher since named twice)
+@@ -314,6 +380,8 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                                        base=self.ou,
+                                        attrs=[])
+         for child in children:
++            self.assertIn(self.mod_inherits_as,
++                          self.sd_utils_dc2.get_sd_as_sddl(child.dn))
+             self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn),
+                               self.sd_utils_dc2.get_sd_as_sddl(child.dn))
+ 
+@@ -322,6 +390,11 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
+                                 fromDC=self.dnsname_dc2,
+                                 forced=True)
+ 
++        self.assertIn(self.mod_inherits_as,
++                      self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn))
++
+         for child in children:
++            self.assertIn(self.mod_inherits_as,
++                          self.sd_utils_dc1.get_sd_as_sddl(child.dn))
+             self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(child.dn),
+                               self.sd_utils_dc2.get_sd_as_sddl(child.dn))
+-- 
+2.17.1
+
+
+From 17215b36b22d309a58a3b7bd08123f06e89657c9 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Tue, 26 Nov 2019 15:44:32 +1300
+Subject: [PATCH 04/13] CVE-2019-14902 dsdb: Explain that
+ descriptor_sd_propagation_recursive() is proctected by a transaction
+
+This means we can trust the DB did not change between the two search
+requests.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ source4/dsdb/samdb/ldb_modules/descriptor.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
+index 9018b750ab5..fb2854438e1 100644
+--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
++++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
+@@ -1199,6 +1199,9 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module,
+ 	 * LDB_SCOPE_SUBTREE searches are expensive.
+ 	 *
+ 	 * Note: that we do not search for deleted/recycled objects
++	 *
++	 * We know this is safe against a rename race as we are in the
++	 * prepare_commit(), so must be in a transaction.
+ 	 */
+ 	ret = dsdb_module_search(module,
+ 				 change,
+-- 
+2.17.1
+
+
+From 589d1e4846bbac0e5388af3ef0c6d6c41b5ff991 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Tue, 26 Nov 2019 16:17:32 +1300
+Subject: [PATCH 05/13] CVE-2019-14902 dsdb: Add comments explaining why SD
+ propagation needs to be done here
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ source4/dsdb/samdb/ldb_modules/descriptor.c | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
+index fb2854438e1..7070affa645 100644
+--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
++++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
+@@ -876,6 +876,9 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
+ 			return ldb_oom(ldb);
+ 		}
+ 
++		/*
++		 * Force SD propagation on children of this record
++		 */
+ 		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
+ 							  dn, false);
+ 		if (ret != LDB_SUCCESS) {
+@@ -966,6 +969,10 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req)
+ 			return ldb_oom(ldb);
+ 		}
+ 
++		/*
++		 * Force SD propagation on this record (get a new
++		 * inherited SD from the potentially new parent
++		 */
+ 		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
+ 							  newdn, true);
+ 		if (ret != LDB_SUCCESS) {
+-- 
+2.17.1
+
+
+From 0fa9a362e55abb289cbf0fe24baa09c45af4837e Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Fri, 6 Dec 2019 17:54:23 +1300
+Subject: [PATCH 06/13] CVE-2019-14902 dsdb: Ensure we honour both
+ change->force_self and change->force_children
+
+If we are renaming a DN we can be in a situation where we need to
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ source4/dsdb/samdb/ldb_modules/descriptor.c | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
+index 7070affa645..b9f465fc36f 100644
+--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
++++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
+@@ -1291,6 +1291,13 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module,
+ 
+ 		if (cur != NULL) {
+ 			DLIST_REMOVE(change->children, cur);
++		} else if (i == 0) {
++			/*
++			 * in the change->force_self case
++			 * res->msgs[0]->elements was not overwritten,
++			 * so set cur here
++			 */
++			cur = change;
+ 		}
+ 
+ 		for (c = stopped_stack; c; c = stopped_stack) {
+-- 
+2.17.1
+
+
+From 9ac2b09fa5a2de44967a0b190918825e7dca8d53 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Fri, 6 Dec 2019 18:05:54 +1300
+Subject: [PATCH 07/13] CVE-2019-14902 repl_meta_data: schedule SD propagation
+ to a renamed DN
+
+We need to check the SD of the parent if we rename, it is not the same as an incoming SD change.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 17 ++++++++++++++++-
+ 1 file changed, 16 insertions(+), 1 deletion(-)
+
+diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+index 04a51ecab51..52ff3d75ee2 100644
+--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
++++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+@@ -6290,7 +6290,22 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
+ 		  ar->index_current, msg->num_elements);
+ 
+ 	if (renamed) {
+-		sd_updated = true;
++		/*
++		 * This is an new name for this object, so we must
++		 * inherit from the parent
++		 *
++		 * This is needed because descriptor is above
++		 * repl_meta_data in the module stack, so this will
++		 * not be trigered 'naturally' by the flow of
++		 * operations.
++		 */
++		ret = dsdb_module_schedule_sd_propagation(ar->module,
++							  ar->objs->partition_dn,
++							  msg->dn,
++							  true);
++		if (ret != LDB_SUCCESS) {
++			return ldb_operr(ldb);
++		}
+ 	}
+ 
+ 	if (sd_updated && !isDeleted) {
+-- 
+2.17.1
+
+
+From 9e6b09e0fd52c664de7f0589074fef872c753fa2 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Tue, 26 Nov 2019 15:50:35 +1300
+Subject: [PATCH 08/13] CVE-2019-14902 repl_meta_data: Fix issue where
+ inherited Security Descriptors were not replicated.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ selftest/knownfail.d/repl_secdesc             |  1 -
+ .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 22 ++++++++++++++++++-
+ 2 files changed, 21 insertions(+), 2 deletions(-)
+
+diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc
+index 7d554ff237a..13a9ce458dd 100644
+--- a/selftest/knownfail.d/repl_secdesc
++++ b/selftest/knownfail.d/repl_secdesc
+@@ -1,3 +1,2 @@
+ ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_object_in_conflict
+-^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inherit_existing_object
+ ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_child_object
+diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+index 52ff3d75ee2..9812ded99fb 100644
+--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
++++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+@@ -5527,6 +5527,15 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar)
+ 	replmd_ldb_message_sort(msg, ar->schema);
+ 
+ 	if (!remote_isDeleted) {
++		/*
++		 * Ensure any local ACL inheritence is applied from
++		 * the parent object.
++		 *
++		 * This is needed because descriptor is above
++		 * repl_meta_data in the module stack, so this will
++		 * not be trigered 'naturally' by the flow of
++		 * operations.
++		 */
+ 		ret = dsdb_module_schedule_sd_propagation(ar->module,
+ 							  ar->objs->partition_dn,
+ 							  msg->dn, true);
+@@ -6309,9 +6318,20 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
+ 	}
+ 
+ 	if (sd_updated && !isDeleted) {
++		/*
++		 * This is an existing object, so there is no need to
++		 * inherit from the parent, but we must inherit any
++		 * incoming changes to our child objects.
++		 *
++		 * This is needed because descriptor is above
++		 * repl_meta_data in the module stack, so this will
++		 * not be trigered 'naturally' by the flow of
++		 * operations.
++		 */
+ 		ret = dsdb_module_schedule_sd_propagation(ar->module,
+ 							  ar->objs->partition_dn,
+-							  msg->dn, true);
++							  msg->dn,
++							  false);
+ 		if (ret != LDB_SUCCESS) {
+ 			return ldb_operr(ldb);
+ 		}
+-- 
+2.17.1
+
+
+From 7071888d5b556213be79545cac059a8b3f62baee Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Fri, 6 Dec 2019 18:26:42 +1300
+Subject: [PATCH 09/13] CVE-2019-14902 repl_meta_data: Set renamed = true (and
+ so do SD inheritance) after any rename
+
+Previously if there was a conflict, but the incoming object would still
+win, this was not marked as a rename, and so inheritence was not done.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ selftest/knownfail.d/repl_secdesc               |  1 -
+ source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 13 +++++++++++++
+ 2 files changed, 13 insertions(+), 1 deletion(-)
+
+diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc
+index 13a9ce458dd..9dd632d99ed 100644
+--- a/selftest/knownfail.d/repl_secdesc
++++ b/selftest/knownfail.d/repl_secdesc
+@@ -1,2 +1 @@
+-^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_object_in_conflict
+ ^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_child_object
+diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+index 9812ded99fb..e67c3b0281e 100644
+--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
++++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+@@ -6134,6 +6134,19 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
+ 		 * replmd_replicated_apply_search_callback())
+ 		 */
+ 		ret = replmd_replicated_handle_rename(ar, msg, ar->req, &renamed);
++
++		/*
++		 * This looks strange, but we must set this after any
++		 * rename, otherwise the SD propegation will not
++		 * happen (which might matter if we have a new parent)
++		 *
++		 * The additional case of calling
++		 * replmd_op_name_modify_callback (below) is:
++		 *  - a no-op if there was no name change
++		 * and
++		 *  - called in the default case regardless.
++		 */
++		renamed = true;
+ 	}
+ 
+ 	if (ret != LDB_SUCCESS) {
+-- 
+2.17.1
+
+
+From 16b377276ee82c04d069666e53deaa95a7633dd4 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Thu, 12 Dec 2019 14:44:57 +1300
+Subject: [PATCH 10/13] CVE-2019-14902 dsdb: Change basis of descriptor module
+ deferred processing to be GUIDs
+
+We can not process on the basis of a DN, as the DN may have changed in a rename,
+not only that this module can see, but also from repl_meta_data below.
+
+Therefore remove all the complex tree-based change processing, leaving only
+a tree-based sort of the possible objects to be changed, and a single
+stopped_dn variable containing the DN to stop processing below (after
+a no-op change).
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
+
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ selftest/knownfail.d/repl_secdesc             |   1 -
+ source4/dsdb/samdb/ldb_modules/acl_util.c     |   4 +-
+ source4/dsdb/samdb/ldb_modules/descriptor.c   | 296 +++++++++---------
+ .../dsdb/samdb/ldb_modules/repl_meta_data.c   |   7 +-
+ source4/dsdb/samdb/samdb.h                    |   2 +-
+ 5 files changed, 156 insertions(+), 154 deletions(-)
+ delete mode 100644 selftest/knownfail.d/repl_secdesc
+
+diff --git a/selftest/knownfail.d/repl_secdesc b/selftest/knownfail.d/repl_secdesc
+deleted file mode 100644
+index 9dd632d99ed..00000000000
+--- a/selftest/knownfail.d/repl_secdesc
++++ /dev/null
+@@ -1 +0,0 @@
+-^samba4.drs.repl_secdesc.python\(.*\).repl_secdesc.ReplAclTestCase.test_acl_inheirt_renamed_child_object
+diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c
+index 6d645b10fe2..b9931795e19 100644
+--- a/source4/dsdb/samdb/ldb_modules/acl_util.c
++++ b/source4/dsdb/samdb/ldb_modules/acl_util.c
+@@ -286,7 +286,7 @@ uint32_t dsdb_request_sd_flags(struct ldb_request *req, bool *explicit)
+ 
+ int dsdb_module_schedule_sd_propagation(struct ldb_module *module,
+ 					struct ldb_dn *nc_root,
+-					struct ldb_dn *dn,
++					struct GUID guid,
+ 					bool include_self)
+ {
+ 	struct ldb_context *ldb = ldb_module_get_ctx(module);
+@@ -299,7 +299,7 @@ int dsdb_module_schedule_sd_propagation(struct ldb_module *module,
+ 	}
+ 
+ 	op->nc_root = nc_root;
+-	op->dn = dn;
++	op->guid = guid;
+ 	op->include_self = include_self;
+ 
+ 	ret = dsdb_module_extended(module, op, NULL,
+diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
+index b9f465fc36f..daa08c2ebc7 100644
+--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
++++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
+@@ -46,9 +46,8 @@
+ 
+ struct descriptor_changes {
+ 	struct descriptor_changes *prev, *next;
+-	struct descriptor_changes *children;
+ 	struct ldb_dn *nc_root;
+-	struct ldb_dn *dn;
++	struct GUID guid;
+ 	bool force_self;
+ 	bool force_children;
+ 	struct ldb_dn *stopped_dn;
+@@ -771,7 +770,8 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
+ 				    current_attrs,
+ 				    DSDB_FLAG_NEXT_MODULE |
+ 				    DSDB_FLAG_AS_SYSTEM |
+-				    DSDB_SEARCH_SHOW_RECYCLED,
++				    DSDB_SEARCH_SHOW_RECYCLED |
++				    DSDB_SEARCH_SHOW_EXTENDED_DN,
+ 				    req);
+ 	if (ret != LDB_SUCCESS) {
+ 		ldb_debug(ldb, LDB_DEBUG_ERROR,"descriptor_modify: Could not find %s\n",
+@@ -832,7 +832,7 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
+ 		user_sd = old_sd;
+ 	}
+ 
+-	sd = get_new_descriptor(module, dn, req,
++	sd = get_new_descriptor(module, current_res->msgs[0]->dn, req,
+ 				objectclass, parent_sd,
+ 				user_sd, old_sd, sd_flags);
+ 	if (sd == NULL) {
+@@ -869,18 +869,32 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
+ 			return ldb_oom(ldb);
+ 		}
+ 	} else if (cmp_ret != 0) {
++		struct GUID guid;
+ 		struct ldb_dn *nc_root;
++		NTSTATUS status;
+ 
+-		ret = dsdb_find_nc_root(ldb, msg, dn, &nc_root);
++		ret = dsdb_find_nc_root(ldb,
++					msg,
++					current_res->msgs[0]->dn,
++					&nc_root);
+ 		if (ret != LDB_SUCCESS) {
+ 			return ldb_oom(ldb);
+ 		}
+ 
++		status = dsdb_get_extended_dn_guid(current_res->msgs[0]->dn,
++						   &guid,
++						   "GUID");
++		if (!NT_STATUS_IS_OK(status)) {
++			return ldb_operr(ldb);
++		}
++
+ 		/*
+ 		 * Force SD propagation on children of this record
+ 		 */
+-		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
+-							  dn, false);
++		ret = dsdb_module_schedule_sd_propagation(module,
++							  nc_root,
++							  guid,
++							  false);
+ 		if (ret != LDB_SUCCESS) {
+ 			return ldb_operr(ldb);
+ 		}
+@@ -963,20 +977,31 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req)
+ 
+ 	if (ldb_dn_compare(olddn, newdn) != 0) {
+ 		struct ldb_dn *nc_root;
++		struct GUID guid;
+ 
+ 		ret = dsdb_find_nc_root(ldb, req, newdn, &nc_root);
+ 		if (ret != LDB_SUCCESS) {
+ 			return ldb_oom(ldb);
+ 		}
+ 
+-		/*
+-		 * Force SD propagation on this record (get a new
+-		 * inherited SD from the potentially new parent
+-		 */
+-		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
+-							  newdn, true);
+-		if (ret != LDB_SUCCESS) {
+-			return ldb_operr(ldb);
++		ret = dsdb_module_guid_by_dn(module,
++					     olddn,
++					     &guid,
++					     req);
++		if (ret == LDB_SUCCESS) {
++			/*
++			 * Without disturbing any errors if the olddn
++			 * does not exit, force SD propagation on
++			 * this record (get a new inherited SD from
++			 * the potentially new parent
++			 */
++			ret = dsdb_module_schedule_sd_propagation(module,
++								  nc_root,
++								  guid,
++								  true);
++			if (ret != LDB_SUCCESS) {
++				return ldb_operr(ldb);
++			}
+ 		}
+ 	}
+ 
+@@ -992,9 +1017,7 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module,
+ 	struct ldb_context *ldb = ldb_module_get_ctx(module);
+ 	struct dsdb_extended_sec_desc_propagation_op *op;
+ 	TALLOC_CTX *parent_mem = NULL;
+-	struct descriptor_changes *parent_change = NULL;
+ 	struct descriptor_changes *c;
+-	int ret;
+ 
+ 	op = talloc_get_type(req->op.extended.data,
+ 			     struct dsdb_extended_sec_desc_propagation_op);
+@@ -1011,32 +1034,6 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module,
+ 
+ 	parent_mem = descriptor_private->trans_mem;
+ 
+-	for (c = descriptor_private->changes; c; c = c->next) {
+-		ret = ldb_dn_compare(c->nc_root, op->nc_root);
+-		if (ret != 0) {
+-			continue;
+-		}
+-
+-		ret = ldb_dn_compare(c->dn, op->dn);
+-		if (ret == 0) {
+-			if (op->include_self) {
+-				c->force_self = true;
+-			} else {
+-				c->force_children = true;
+-			}
+-			return ldb_module_done(req, NULL, NULL, LDB_SUCCESS);
+-		}
+-
+-		ret = ldb_dn_compare_base(c->dn, op->dn);
+-		if (ret != 0) {
+-			continue;
+-		}
+-
+-		parent_mem = c;
+-		parent_change = c;
+-		break;
+-	}
+-
+ 	c = talloc_zero(parent_mem, struct descriptor_changes);
+ 	if (c == NULL) {
+ 		return ldb_module_oom(module);
+@@ -1045,21 +1042,14 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module,
+ 	if (c->nc_root == NULL) {
+ 		return ldb_module_oom(module);
+ 	}
+-	c->dn = ldb_dn_copy(c, op->dn);
+-	if (c->dn == NULL) {
+-		return ldb_module_oom(module);
+-	}
++	c->guid = op->guid;
+ 	if (op->include_self) {
+ 		c->force_self = true;
+ 	} else {
+ 		c->force_children = true;
+ 	}
+ 
+-	if (parent_change != NULL) {
+-		DLIST_ADD_END(parent_change->children, c);
+-	} else {
+-		DLIST_ADD_END(descriptor_private->changes, c);
+-	}
++	DLIST_ADD_END(descriptor_private->changes, c);
+ 
+ 	return ldb_module_done(req, NULL, NULL, LDB_SUCCESS);
+ }
+@@ -1179,41 +1169,75 @@ static int descriptor_sd_propagation_msg_sort(struct ldb_message **m1,
+ 	return ldb_dn_compare(dn2, dn1);
+ }
+ 
+-static int descriptor_sd_propagation_dn_sort(struct ldb_dn *dn1,
+-					     struct ldb_dn *dn2)
+-{
+-	/*
+-	 * This sorts in tree order, parents first
+-	 */
+-	return ldb_dn_compare(dn2, dn1);
+-}
+-
+ static int descriptor_sd_propagation_recursive(struct ldb_module *module,
+ 					       struct descriptor_changes *change)
+ {
+-	struct ldb_context *ldb = ldb_module_get_ctx(module);
++	struct ldb_result *guid_res = NULL;
+ 	struct ldb_result *res = NULL;
+ 	unsigned int i;
+ 	const char * const no_attrs[] = { "@__NONE__", NULL };
+-	struct descriptor_changes *c;
+-	struct descriptor_changes *stopped_stack = NULL;
+-	enum ldb_scope scope;
++	struct ldb_dn *stopped_dn = NULL;
++	struct GUID_txt_buf guid_buf;
+ 	int ret;
++	bool stop = false;
+ 
+ 	/*
+-	 * First confirm this object has children, or exists (depending on change->force_self)
++	 * First confirm this object has children, or exists
++	 * (depending on change->force_self)
+ 	 * 
+ 	 * LDB_SCOPE_SUBTREE searches are expensive.
+ 	 *
+-	 * Note: that we do not search for deleted/recycled objects
+-	 *
+ 	 * We know this is safe against a rename race as we are in the
+ 	 * prepare_commit(), so must be in a transaction.
+ 	 */
++
++	/* Find the DN by GUID, as this is stable under rename */
++	ret = dsdb_module_search(module,
++				 change,
++				 &guid_res,
++				 change->nc_root,
++				 LDB_SCOPE_SUBTREE,
++				 no_attrs,
++				 DSDB_FLAG_NEXT_MODULE |
++				 DSDB_FLAG_AS_SYSTEM |
++				 DSDB_SEARCH_SHOW_DELETED |
++				 DSDB_SEARCH_SHOW_RECYCLED,
++				 NULL, /* parent_req */
++				 "(objectGUID=%s)",
++				 GUID_buf_string(&change->guid,
++						 &guid_buf));
++
++	if (ret != LDB_SUCCESS) {
++		return ret;
++	}
++
++	if (guid_res->count != 1) {
++		/*
++		 * We were just given this GUID during the same
++		 * transaction, if it is missing this is a big
++		 * problem.
++		 *
++		 * Cleanup of tombstones does not trigger this module
++		 * as it just does a delete.
++		 */
++		ldb_asprintf_errstring(ldb_module_get_ctx(module),
++				       "failed to find GUID %s under %s "
++				       "for transaction-end SD inheritance: %d results",
++				       GUID_buf_string(&change->guid,
++						       &guid_buf),
++				       ldb_dn_get_linearized(change->nc_root),
++				       guid_res->count);
++		return LDB_ERR_OPERATIONS_ERROR;
++	}
++
++	/*
++	 * OK, so there was a parent, are there children?  Note: that
++	 * this time we do not search for deleted/recycled objects
++	 */
+ 	ret = dsdb_module_search(module,
+ 				 change,
+ 				 &res,
+-				 change->dn,
++				 guid_res->msgs[0]->dn,
+ 				 LDB_SCOPE_ONELEVEL,
+ 				 no_attrs,
+ 				 DSDB_FLAG_NEXT_MODULE |
+@@ -1221,26 +1245,55 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module,
+ 				 NULL, /* parent_req */
+ 				 "(objectClass=*)");
+ 	if (ret != LDB_SUCCESS) {
++		/*
++		 * LDB_ERR_NO_SUCH_OBJECT, say if the DN was a deleted
++		 * object, is ignored by the caller
++		 */
+ 		return ret;
+ 	}
+ 
+ 	if (res->count == 0 && !change->force_self) {
++		/* All done, no children */
+ 		TALLOC_FREE(res);
+ 		return LDB_SUCCESS;
+-	} else if (res->count == 0 && change->force_self) {
+-		scope = LDB_SCOPE_BASE;
+-	} else {
+-		scope = LDB_SCOPE_SUBTREE;
+ 	}
+ 
+ 	/*
++	 * First, if we are in force_self mode (eg renamed under new
++	 * parent) then apply the SD to the top object
++	 */
++	if (change->force_self) {
++		ret = descriptor_sd_propagation_object(module,
++						       guid_res->msgs[0],
++						       &stop);
++		if (ret != LDB_SUCCESS) {
++			TALLOC_FREE(guid_res);
++			return ret;
++		}
++
++		if (stop == true && !change->force_children) {
++			/* There was no change, nothing more to do */
++			TALLOC_FREE(guid_res);
++			return LDB_SUCCESS;
++		}
++
++		if (res->count == 0) {
++			/* All done! */
++			TALLOC_FREE(guid_res);
++			return LDB_SUCCESS;
++		}
++	}
++
++	/*
++	 * Look for children
++	 *
+ 	 * Note: that we do not search for deleted/recycled objects
+ 	 */
+ 	ret = dsdb_module_search(module,
+ 				 change,
+ 				 &res,
+-				 change->dn,
+-				 scope,
++				 guid_res->msgs[0]->dn,
++				 LDB_SCOPE_SUBTREE,
+ 				 no_attrs,
+ 				 DSDB_FLAG_NEXT_MODULE |
+ 				 DSDB_FLAG_AS_SYSTEM,
+@@ -1253,90 +1306,39 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module,
+ 	TYPESAFE_QSORT(res->msgs, res->count,
+ 		       descriptor_sd_propagation_msg_sort);
+ 
+-	for (c = change->children; c; c = c->next) {
+-		struct ldb_message *msg = NULL;
+-
+-		BINARY_ARRAY_SEARCH_P(res->msgs, res->count, dn, c->dn,
+-				      descriptor_sd_propagation_dn_sort,
+-				      msg);
+-
+-		if (msg == NULL) {
+-			ldb_debug(ldb, LDB_DEBUG_WARNING,
+-				"descriptor_sd_propagation_recursive: "
+-				"%s not found under %s",
+-				ldb_dn_get_linearized(c->dn),
+-				ldb_dn_get_linearized(change->dn));
+-			continue;
+-		}
+-
+-		msg->elements = (struct ldb_message_element *)c;
+-	}
+-
+-	DLIST_ADD(stopped_stack, change);
+-
+-	if (change->force_self) {
+-		i = 0;
+-	} else {
+-		i = 1;
+-	}
+-
+-	for (; i < res->count; i++) {
+-		struct descriptor_changes *cur;
+-		bool stop = false;
+-
+-		cur = talloc_get_type(res->msgs[i]->elements,
+-				      struct descriptor_changes);
+-		res->msgs[i]->elements = NULL;
+-		res->msgs[i]->num_elements = 0;
+-
+-		if (cur != NULL) {
+-			DLIST_REMOVE(change->children, cur);
+-		} else if (i == 0) {
++	/* We start from 1, the top object has been done */
++	for (i = 1; i < res->count; i++) {
++		/*
++		 * ldb_dn_compare_base() does not match for NULL but
++		 * this is clearer
++		 */
++		if (stopped_dn != NULL) {
++			ret = ldb_dn_compare_base(stopped_dn,
++						  res->msgs[i]->dn);
+ 			/*
+-			 * in the change->force_self case
+-			 * res->msgs[0]->elements was not overwritten,
+-			 * so set cur here
++			 * Skip further processing of this
++			 * sub-subtree
+ 			 */
+-			cur = change;
+-		}
+-
+-		for (c = stopped_stack; c; c = stopped_stack) {
+-			ret = ldb_dn_compare_base(c->dn,
+-						  res->msgs[i]->dn);
+-			if (ret == 0) {
+-				break;
+-			}
+-
+-			c->stopped_dn = NULL;
+-			DLIST_REMOVE(stopped_stack, c);
+-		}
+-
+-		if (cur != NULL) {
+-			DLIST_ADD(stopped_stack, cur);
+-		}
+-
+-		if (stopped_stack->stopped_dn != NULL) {
+-			ret = ldb_dn_compare_base(stopped_stack->stopped_dn,
+-						  res->msgs[i]->dn);
+ 			if (ret == 0) {
+ 				continue;
+ 			}
+-			stopped_stack->stopped_dn = NULL;
+ 		}
+-
+-		ret = descriptor_sd_propagation_object(module, res->msgs[i],
++		ret = descriptor_sd_propagation_object(module,
++						       res->msgs[i],
+ 						       &stop);
+ 		if (ret != LDB_SUCCESS) {
+ 			return ret;
+ 		}
+ 
+-		if (cur != NULL && cur->force_children) {
+-			continue;
+-		}
+-
+ 		if (stop) {
+-			stopped_stack->stopped_dn = res->msgs[i]->dn;
+-			continue;
++			/*
++			 * If this child didn't change, then nothing
++			 * under it needs to change
++			 *
++			 * res has been sorted into tree order so the
++			 * next few entries can be skipped
++			 */
++			stopped_dn = res->msgs[i]->dn;
+ 		}
+ 	}
+ 
+diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+index e67c3b0281e..a2a6bcc98f3 100644
+--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
++++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+@@ -5538,7 +5538,8 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar)
+ 		 */
+ 		ret = dsdb_module_schedule_sd_propagation(ar->module,
+ 							  ar->objs->partition_dn,
+-							  msg->dn, true);
++							  ar->objs->objects[ar->index_current].object_guid,
++							  true);
+ 		if (ret != LDB_SUCCESS) {
+ 			return replmd_replicated_request_error(ar, ret);
+ 		}
+@@ -6323,7 +6324,7 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
+ 		 */
+ 		ret = dsdb_module_schedule_sd_propagation(ar->module,
+ 							  ar->objs->partition_dn,
+-							  msg->dn,
++							  ar->objs->objects[ar->index_current].object_guid,
+ 							  true);
+ 		if (ret != LDB_SUCCESS) {
+ 			return ldb_operr(ldb);
+@@ -6343,7 +6344,7 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
+ 		 */
+ 		ret = dsdb_module_schedule_sd_propagation(ar->module,
+ 							  ar->objs->partition_dn,
+-							  msg->dn,
++							  ar->objs->objects[ar->index_current].object_guid,
+ 							  false);
+ 		if (ret != LDB_SUCCESS) {
+ 			return ldb_operr(ldb);
+diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
+index e1b0e4aa4e3..3f47b863a83 100644
+--- a/source4/dsdb/samdb/samdb.h
++++ b/source4/dsdb/samdb/samdb.h
+@@ -338,7 +338,7 @@ struct dsdb_extended_allocate_rid {
+ #define DSDB_EXTENDED_SEC_DESC_PROPAGATION_OID "1.3.6.1.4.1.7165.4.4.7"
+ struct dsdb_extended_sec_desc_propagation_op {
+ 	struct ldb_dn *nc_root;
+-	struct ldb_dn *dn;
++	struct GUID guid;
+ 	bool include_self;
+ };
+ 
+-- 
+2.17.1
+
+
+From 030fa9e5455125e30b71c90be80baadb657d8993 Mon Sep 17 00:00:00 2001
+From: Noel Power <noel.power@suse.com>
+Date: Fri, 24 May 2019 13:37:00 +0000
+Subject: [PATCH 11/13] CVE-2019-14907 lib/util/charset: clang: Fix Value
+ stored to 'reason' is never read warning
+
+Fixes:
+
+lib/util/charset/convert_string.c:301:5: warning: Value stored to 'reason' is never read <--[clang]
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=14208
+
+Signed-off-by: Noel Power <noel.power@suse.com>
+Reviewed-by: Gary Lockyer gary@catalyst.net.nz
+(cherry picked from commit add47e288bc80c1bf45765d1588a9fa5998ea677)
+---
+ lib/util/charset/convert_string.c | 8 ++++----
+ 1 file changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/lib/util/charset/convert_string.c b/lib/util/charset/convert_string.c
+index 196302aacfd..34facab6fe6 100644
+--- a/lib/util/charset/convert_string.c
++++ b/lib/util/charset/convert_string.c
+@@ -300,13 +300,13 @@ bool convert_string_handle(struct smb_iconv_handle *ic,
+ 			{
+ 				reason="No more room";
+ 				if (from == CH_UNIX) {
+-					DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u - '%s'\n",
++					DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u - '%s' error: %s\n",
+ 						 charset_name(ic, from), charset_name(ic, to),
+-						 (unsigned int)srclen, (unsigned int)destlen, (const char *)src));
++						 (unsigned int)srclen, (unsigned int)destlen, (const char *)src, reason));
+ 				} else {
+-					DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u\n",
++					DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n",
+ 						 charset_name(ic, from), charset_name(ic, to),
+-						 (unsigned int)srclen, (unsigned int)destlen));
++						 (unsigned int)srclen, (unsigned int)destlen, reason));
+ 				}
+ 				break;
+ 			}
+-- 
+2.17.1
+
+
+From ad0e68d354ad33c577dbf146fc4a1b8254857558 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Fri, 29 Nov 2019 20:58:47 +1300
+Subject: [PATCH 12/13] CVE-2019-14907 lib/util: Do not print the failed to
+ convert string into the logs
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+The string may be in another charset, or may be sensitive and
+certainly may not be terminated.  It is not safe to just print.
+
+Found by Robert Święcki using a fuzzer he wrote for smbd.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=14208
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+
+(adapted from master commit)
+---
+ lib/util/charset/convert_string.c | 33 +++++++++++++++++--------------
+ 1 file changed, 18 insertions(+), 15 deletions(-)
+
+diff --git a/lib/util/charset/convert_string.c b/lib/util/charset/convert_string.c
+index 34facab6fe6..b546e056953 100644
+--- a/lib/util/charset/convert_string.c
++++ b/lib/util/charset/convert_string.c
+@@ -293,31 +293,31 @@ bool convert_string_handle(struct smb_iconv_handle *ic,
+ 		switch(errno) {
+ 			case EINVAL:
+ 				reason="Incomplete multibyte sequence";
+-				DEBUG(3,("convert_string_internal: Conversion error: %s(%s)\n",
+-					 reason, (const char *)src));
++				DBG_NOTICE("Conversion error: %s\n",
++					 reason);
+ 				break;
+ 			case E2BIG:
+ 			{
+ 				reason="No more room";
+ 				if (from == CH_UNIX) {
+-					DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u - '%s' error: %s\n",
+-						 charset_name(ic, from), charset_name(ic, to),
+-						 (unsigned int)srclen, (unsigned int)destlen, (const char *)src, reason));
++					DBG_NOTICE("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n",
++						   charset_name(ic, from), charset_name(ic, to),
++						   (unsigned int)srclen, (unsigned int)destlen, reason);
+ 				} else {
+-					DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n",
+-						 charset_name(ic, from), charset_name(ic, to),
+-						 (unsigned int)srclen, (unsigned int)destlen, reason));
++					DBG_NOTICE("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n",
++						   charset_name(ic, from), charset_name(ic, to),
++						   (unsigned int)srclen, (unsigned int)destlen, reason);
+ 				}
+ 				break;
+ 			}
+ 			case EILSEQ:
+ 				reason="Illegal multibyte sequence";
+-				DEBUG(3,("convert_string_internal: Conversion error: %s(%s)\n",
+-					 reason, (const char *)src));
++				DBG_NOTICE("convert_string_internal: Conversion error: %s\n",
++					   reason);
+ 				break;
+ 			default:
+-				DEBUG(0,("convert_string_internal: Conversion error: %s(%s)\n",
+-					 reason, (const char *)src));
++				DBG_ERR("convert_string_internal: Conversion error: %s\n",
++					reason);
+ 				break;
+ 		}
+ 		/* smb_panic(reason); */
+@@ -427,16 +427,19 @@ bool convert_string_talloc_handle(TALLOC_CTX *ctx, struct smb_iconv_handle *ic,
+ 		switch(errno) {
+ 			case EINVAL:
+ 				reason="Incomplete multibyte sequence";
+-				DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf));
++				DBG_NOTICE("Conversion error: %s\n",
++					   reason);
+ 				break;
+ 			case E2BIG:
+ 				goto convert;
+ 			case EILSEQ:
+ 				reason="Illegal multibyte sequence";
+-				DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf));
++				DBG_NOTICE("Conversion error: %s\n",
++					   reason);
+ 				break;
+ 			default:
+-				DEBUG(0,("Conversion error: %s(%s)\n",reason,inbuf));
++				DBG_ERR("Conversion error: %s\n",
++					reason);
+ 				break;
+ 		}
+ 		/* smb_panic(reason); */
+-- 
+2.17.1
+
+