Attachment 16461 Details for Bug 14595 – patch for 4.14 v2

[patch] patch for 4.14 v2

CVE-2020-27840-patch-for-4-14-v2.patch (text/plain), 9.18 KB, created by Douglas Bagnall on 2021-02-16 22:40:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Douglas Bagnall

Created: 2021-02-16 22:40:15 UTC

Size: 9.18 KB

patch

obsolete

>From e1d4db4a2db077f9f4bdcc794b4a43ccbbcaf002 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 07e2d46d56a..70bd5b180e3 100644
>--- a/python/samba/tests/segfault.py
>+++ b/python/samba/tests/segfault.py
>@@ -174,3 +174,9 @@ class SegfaultTests(samba.tests.TestCase):
>     def test_dcerpc_idl_inline_arrays(self):
>         """Inline arrays were incorrectly handled."""
>         dnsserver.DNS_RPC_SERVER_INFO_DOTNET().pExtensions
>+
>+    @segfault_detector
>+    def test_ldb_dn_explode_crash(self):
>+        for i in range(106, 550, 5):
>+            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 1be0566dcb1..524f7dd013b 100644
>--- a/selftest/knownfail.d/python-segfaults
>+++ b/selftest/knownfail.d/python-segfaults
>@@ -1 +1,2 @@
> 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 da1cb77df08a181efa38856a91fa2507c497246c 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 001fcad621f..cce5ad5b2ff 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 524f7dd013b..1be0566dcb1 100644
>--- a/selftest/knownfail.d/python-segfaults
>+++ b/selftest/knownfail.d/python-segfaults
>@@ -1,2 +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 082121c78cbd229e4e9022a4fccb2483699183cc 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 f374f64aeab..32a1a2e0ec0 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 70bd5b180e3..07e2d46d56a 100644
>--- a/python/samba/tests/segfault.py
>+++ b/python/samba/tests/segfault.py
>@@ -174,9 +174,3 @@ class SegfaultTests(samba.tests.TestCase):
>     def test_dcerpc_idl_inline_arrays(self):
>         """Inline arrays were incorrectly handled."""
>         dnsserver.DNS_RPC_SERVER_INFO_DOTNET().pExtensions
>-
>-    @segfault_detector
>-    def test_ldb_dn_explode_crash(self):
>-        for i in range(106, 550, 5):
>-            dn = ldb.Dn(ldb.Ldb(), "a=b%s,c= " % (' ' * i))
>-            dn.validate()
>-- 
>2.25.1
>

Flags:

abartlet: review+
dbagnall: ci-passed+

Actions: View

Attachments on bug 14595: 16365 | 16366 | 16383 | 16441 | 16442 | 16444 | 16445 | 16446 | 16447 | 16448 | 16449 | 16460 | 16461 | 16462 | 16463 | 16464 | 16530 | 16547