From 4f2cf13ea2f290c678c60f756c2668fb54c72240 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Thu, 11 Feb 2021 17:05:14 +1300
Subject: [PATCH 1/3] CVE-2020-27840: pytests:segfault: add ldb.Dn validate
 test

ldb.Dn.validate wraps ldb_dn_explode.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14595

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 python/samba/tests/segfault.py        | 6 ++++++
 selftest/knownfail.d/python-segfaults | 1 +
 2 files changed, 7 insertions(+)

diff --git a/python/samba/tests/segfault.py b/python/samba/tests/segfault.py
index b95c1ef5fa9..7e306a61549 100644
--- a/python/samba/tests/segfault.py
+++ b/python/samba/tests/segfault.py
@@ -134,3 +134,9 @@ class SegfaultTests(samba.tests.TestCase):
         except ldb.LdbError:
             pass
         str(m)
+
+    @segfault_detector
+    def test_ldb_dn_explode_crash(self):
+        for i in range(106, 150):
+            dn = ldb.Dn(ldb.Ldb(), "a=b%s,c= " % (' ' * i))
+            dn.validate()
diff --git a/selftest/knownfail.d/python-segfaults b/selftest/knownfail.d/python-segfaults
index c8331d941ef..5a440437c4d 100644
--- a/selftest/knownfail.d/python-segfaults
+++ b/selftest/knownfail.d/python-segfaults
@@ -4,3 +4,4 @@ samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_hive_open_ldb
 samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_net_replicate_chunk_1
 samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_net_replicate_init__1
 samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_net_replicate_init__3
+samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_dn_explode_crash
-- 
2.25.1


From 8f6086f66f249e7569e7157b479e880988d67547 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 11 Dec 2020 16:32:25 +1300
Subject: [PATCH 2/3] CVE-2020-27840 ldb_dn: avoid head corruption in
 ldb_dn_explode

A DN string with lots of trailing space can cause ldb_dn_explode() to
put a zero byte in the wrong place in the heap.

When a DN string has a value represented with trailing spaces,
like this

     "CN=foo   ,DC=bar"

the whitespace is supposed to be ignored. We keep track of this in the
`t` pointer, which is NULL when we are not walking through trailing
spaces, and points to the first space when we are. We are walking with
the `p` pointer, writing the value to `d`, and keeping the length in
`l`.

     "CN=foo   ,DC= "       ==>       "foo   "
            ^  ^                             ^
            t  p                             d
                                       --l---

The value is finished when we encounter a comma or the end of the
string. If `t` is not NULL at that point, we assume there are trailing
spaces and wind `d and `l` back by the correct amount. Then we switch
to expecting an attribute name (e.g. "CN"), until we get to an "=",
which puts us back into looking for a value.

Unfortunately, we forget to immediately tell `t` that we'd finished
the last value, we can end up like this:

     "CN=foo   ,DC= "       ==>        ""
            ^      ^                    ^
            t      p                    d
                                        l=0

where `p` is pointing to a new value that contains only spaces, while
`t` is still referring to the old value. `p` notices the value ends,
and we subtract `p - t` from `d`:

     "CN=foo   ,DC= "       ==>  ?     ""
            ^       ^            ^
            t       p            d
                                      l ~= SIZE_MAX - 8

At that point `d` wants to terminate its string with a '\0', but
instead it terminates someone else's byte. This does not crash if the
number of trailing spaces is small, as `d` will point into a previous
value (a copy of "foo" in this example). Corrupting that value will
ultimately not matter, as we will soon try to allocate a buffer `l`
long, which will be greater than the available memory and the whole
operation will fail properly.

However, with more spaces, `d` will point into memory before the
beginning of the allocated buffer, with the exact offset depending on
the length of the earlier attributes and the number of spaces.

What about a longer DN with more attributes? For example,
"CN=foo     ,DC= ,DC=example,DC=com" -- since `d` has moved out of
bounds, won't we continue to use it and write more DN values into
mystery memory? Fortunately not, because the aforementioned allocation
of `l` bytes must happen first, and `l` is now huge. The allocation
happens in a talloc_memdup(), which is by default restricted to
allocating 256MB.

So this allows a person who controls a string parsed by ldb_dn_explode
to corrupt heap memory by placing a single zero byte at a chosen
offset before the allocated buffer.

An LDAP bind request can send a string DN as a username. This DN is
necessarily parsed before the password is checked, so an attacker does
not need proper credentials. The attacker can easily cause a denial of
service and we cannot rule out more subtle attacks.

The immediate solution is to reset `t` to NULL when a comma is
encountered, indicating that we are no longer looking at trailing
whitespace.

Found with the help of Honggfuzz.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14595

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 lib/ldb/common/ldb_dn.c               | 1 +
 selftest/knownfail.d/python-segfaults | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
index 83f94e3b913..047244287f5 100644
--- a/lib/ldb/common/ldb_dn.c
+++ b/lib/ldb/common/ldb_dn.c
@@ -570,6 +570,7 @@ static bool ldb_dn_explode(struct ldb_dn *dn)
 					/* trim back */
 					d -= (p - t);
 					l -= (p - t);
+					t = NULL;
 				}
 
 				in_attr = true;
diff --git a/selftest/knownfail.d/python-segfaults b/selftest/knownfail.d/python-segfaults
index 5a440437c4d..c8331d941ef 100644
--- a/selftest/knownfail.d/python-segfaults
+++ b/selftest/knownfail.d/python-segfaults
@@ -4,4 +4,3 @@ samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_hive_open_ldb
 samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_net_replicate_chunk_1
 samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_net_replicate_init__1
 samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_net_replicate_init__3
-samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_dn_explode_crash
-- 
2.25.1


From aecdb95ab26845f0993c3bd7a354ab7554120cab Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Thu, 11 Feb 2021 16:28:43 +1300
Subject: [PATCH 3/3] CVE-2020-27840: pytests: move Dn.validate test to ldb

We had the test in the Samba Python segfault suite because
a) the signal catching infrastructure was there, and
b) the ldb tests lack Samba's knownfail mechanism, which allowed us to
   assert the failure.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14595

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 lib/ldb/tests/python/crash.py  | 45 ++++++++++++++++++++++++++++++++++
 lib/ldb/wscript                |  1 +
 python/samba/tests/segfault.py |  6 -----
 3 files changed, 46 insertions(+), 6 deletions(-)
 create mode 100644 lib/ldb/tests/python/crash.py

diff --git a/lib/ldb/tests/python/crash.py b/lib/ldb/tests/python/crash.py
new file mode 100644
index 00000000000..32839814552
--- /dev/null
+++ b/lib/ldb/tests/python/crash.py
@@ -0,0 +1,45 @@
+#!/usr/bin/env python3
+#
+# Tests for crashing functions
+
+import os
+from unittest import TestCase
+import os
+import sys
+import traceback
+
+import ldb
+
+
+def segfault_detector(f):
+    def wrapper(*args, **kwargs):
+        pid = os.fork()
+        if pid == 0:
+            # child, crashing?
+            try:
+                f(*args, **kwargs)
+            except Exception as e:
+                traceback.print_exc()
+            sys.stderr.flush()
+            sys.stdout.flush()
+            os._exit(0)
+
+        # parent, waiting
+        pid2, status = os.waitpid(pid, 0)
+        if os.WIFSIGNALED(status):
+            signal = os.WTERMSIG(status)
+            raise AssertionError("Failed with signal %d" % signal)
+
+    return wrapper
+
+
+class LdbDnCrashTests(TestCase):
+    @segfault_detector
+    def test_ldb_dn_explode_crash(self):
+        for i in range(106, 150):
+            dn = ldb.Dn(ldb.Ldb(), "a=b%s,c= " % (' ' * i))
+            dn.validate()
+
+if __name__ == '__main__':
+    import unittest
+    unittest.TestProgram()
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index da2b935d102..7e0179419df 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -614,6 +614,7 @@ def test(ctx):
         os.mkdir(tmp_dir)
     pyret = samba_utils.RUN_PYTHON_TESTS(
         ['tests/python/api.py',
+         'tests/python/crash.py',
          'tests/python/index.py',
          'tests/python/repack.py'],
         extra_env={'SELFTEST_PREFIX': test_prefix})
diff --git a/python/samba/tests/segfault.py b/python/samba/tests/segfault.py
index 7e306a61549..b95c1ef5fa9 100644
--- a/python/samba/tests/segfault.py
+++ b/python/samba/tests/segfault.py
@@ -134,9 +134,3 @@ class SegfaultTests(samba.tests.TestCase):
         except ldb.LdbError:
             pass
         str(m)
-
-    @segfault_detector
-    def test_ldb_dn_explode_crash(self):
-        for i in range(106, 150):
-            dn = ldb.Dn(ldb.Ldb(), "a=b%s,c= " % (' ' * i))
-            dn.validate()
-- 
2.25.1

