Security fix for CVE-2024-3657
authorPierre Rogier <progier@redhat.com>
Wed, 17 Apr 2024 16:18:04 +0000 (18:18 +0200)
committerAndrej Shadura <andrewsh@debian.org>
Thu, 16 Jan 2025 16:16:37 +0000 (17:16 +0100)
Description:
A flaw was found in the 389 Directory Server. A specially-crafted LDAP query
can potentially cause a failure on the directory server, leading to a denial
of service.

Fix Description:
The code was modified to avoid a buffer overflow when logging some requests
in the audit log.

References:
- https://nvd.nist.gov/vuln/detail/CVE-2024-3657
- https://access.redhat.com/security/cve/CVE-2024-3657
- https://bugzilla.redhat.com/show_bug.cgi?id=2274401

Gbp-Pq: Name CVE-2024-3657.patch

dirsrvtests/tests/suites/filter/large_filter_test.py
ldap/servers/slapd/back-ldbm/index.c

index 6663c7c0f9beb9a9c021920a1a762baa8ffecb87..ba6fb76e5f8737a809a85e99587eb2685ed4ca1b 100644 (file)
@@ -13,19 +13,29 @@ verify and testing  Filter from a search
 
 import os
 import pytest
+import ldap
 
-from lib389._constants import PW_DM
+from lib389._constants import PW_DM, DEFAULT_SUFFIX, ErrorLog
 from lib389.topologies import topology_st as topo
 from lib389.idm.user import UserAccounts, UserAccount
 from lib389.idm.account import Accounts
 from lib389.backend import Backends
 from lib389.idm.domain import Domain
+from lib389.utils import get_ldapurl_from_serverid
 
 SUFFIX = 'dc=anuj,dc=com'
 
 pytestmark = pytest.mark.tier1
 
 
+def open_new_ldapi_conn(dsinstance):
+    ldapurl, certdir = get_ldapurl_from_serverid(dsinstance)
+    assert 'ldapi://' in ldapurl
+    conn = ldap.initialize(ldapurl)
+    conn.sasl_interactive_bind_s("", ldap.sasl.external())
+    return conn
+
+
 @pytest.fixture(scope="module")
 def _create_entries(request, topo):
     """
@@ -160,6 +170,33 @@ def test_large_filter(topo, _create_entries, real_value):
     assert len(Accounts(conn, SUFFIX).filter(real_value)) == 3
 
 
+def test_long_filter_value(topo):
+    """Exercise large eq filter with dn syntax attributes
+
+        :id: b069ef72-fcc3-11ee-981c-482ae39447e5
+        :setup: Standalone
+        :steps:
+            1. Create a specially crafted LDAP filter and
+               pass the filter to a search query with the special repeating string "a\x1Edmin"
+            2. Pass the filter to a search query with repeating string "aAdmin"
+            3. Pass the filter to a search query with string "*"
+        :expectedresults:
+            1. Search should not result in server crash and the server should be running
+    """
+    inst = topo.standalone
+    conn = open_new_ldapi_conn(inst.serverid)
+    inst.config.loglevel(vals=(ErrorLog.DEFAULT,ErrorLog.TRACE,ErrorLog.SEARCH_FILTER))
+    filter_value = "a\x1Edmin" * 1025
+    conn.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, f'(cn={filter_value})')
+    filter_value = "aAdmin" * 1025
+    conn.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, f'(cn={filter_value})')
+    filter_value = "*"
+    conn.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, f'(cn={filter_value})')
+    inst.config.loglevel(vals=(ErrorLog.DEFAULT,))
+    # Check if server is running
+    assert inst.status()
+
+
 if __name__ == '__main__':
     CURRENT_FILE = os.path.realpath(__file__)
     pytest.main("-s -v %s" % CURRENT_FILE)
index 5b34a1e87935f429aa528ef5fbefc263dce6d8db..0e9c3487d1069dac1461968796ebeb87db1e9c86 100644 (file)
@@ -74,6 +74,32 @@ typedef struct _index_buffer_handle index_buffer_handle;
 #define INDEX_BUFFER_FLAG_SERIALIZE 1
 #define INDEX_BUFFER_FLAG_STATS 2
 
+/*
+ * space needed to encode a byte:
+ *  0x00-0x31 and 0x7f-0xff requires 3 bytes: \xx
+ *  0x22 and 0x5C requires 2 bytes: \" and \\
+ *  other requires 1 byte: c
+ */
+static char encode_size[] = {
+    /* 0x00 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0x10 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0x20 */   1, 1, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+    /* 0x30 */   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+    /* 0x40 */   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+    /* 0x50 */   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 1, 1, 1,
+    /* 0x60 */   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+    /* 0x70 */   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 3,
+    /* 0x80 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0x90 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0xA0 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0xB0 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0xC0 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0xD0 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0xE0 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    /* 0xF0 */   3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+};
+
+
 /* Index buffering functions */
 
 static int
@@ -802,65 +828,46 @@ index_add_mods(
 
 /*
  * Convert a 'struct berval' into a displayable ASCII string
+ * returns the printable string
  */
-
-#define SPECIAL(c) (c < 32 || c > 126 || c == '\\' || c == '"')
-
 const char *
 encode(const struct berval *data, char buf[BUFSIZ])
 {
-    char *s;
-    char *last;
-    if (data == NULL || data->bv_len == 0)
-        return "";
-    last = data->bv_val + data->bv_len - 1;
-    for (s = data->bv_val; s < last; ++s) {
-        if (SPECIAL(*s)) {
-            char *first = data->bv_val;
-            char *bufNext = buf;
-            size_t bufSpace = BUFSIZ - 4;
-            while (1) {
-                /* printf ("%lu bytes ASCII\n", (unsigned long)(s - first)); */
-                if (bufSpace < (size_t)(s - first))
-                    s = first + bufSpace - 1;
-                if (s != first) {
-                    memcpy(bufNext, first, s - first);
-                    bufNext += (s - first);
-                    bufSpace -= (s - first);
-                }
-                do {
-                    if (bufSpace) {
-                        *bufNext++ = '\\';
-                        --bufSpace;
-                    }
-                    if (bufSpace < 2) {
-                        memcpy(bufNext, "..", 2);
-                        bufNext += 2;
-                        goto bail;
-                    }
-                    if (*s == '\\' || *s == '"') {
-                        *bufNext++ = *s;
-                        --bufSpace;
-                    } else {
-                        sprintf(bufNext, "%02x", (unsigned)*(unsigned char *)s);
-                        bufNext += 2;
-                        bufSpace -= 2;
-                    }
-                } while (++s <= last && SPECIAL(*s));
-                if (s > last)
-                    break;
-                first = s;
-                while (!SPECIAL(*s) && s <= last)
-                    ++s;
-            }
-        bail:
-            *bufNext = '\0';
-            /* printf ("%lu chars in buffer\n", (unsigned long)(bufNext - buf)); */
+    if (!data || !data->bv_val) {
+        strcpy(buf, "<NULL>");
+        return buf;
+    }
+    char *endbuff = &buf[BUFSIZ-4];  /* Reserve space to append "...\0" */
+    char *ptout = buf;
+    unsigned char *ptin = (unsigned char*) data->bv_val;
+    unsigned char *endptin = ptin+data->bv_len;
+
+    while (ptin < endptin) {
+        if (ptout >= endbuff) {
+            /*
+             * BUFSIZ(8K) > SLAPI_LOG_BUFSIZ(2K) so the error log message will be
+             * truncated anyway. So there is no real interrest to test if the original
+             * data contains no special characters and return it as is.
+             */
+            strcpy(endbuff, "...");
             return buf;
         }
+        switch (encode_size[*ptin]) {
+            case 1:
+                *ptout++ = *ptin++;
+                break;
+            case 2:
+                *ptout++ = '\\';
+                *ptout++ = *ptin++;
+                break;
+            case 3:
+                sprintf(ptout, "\\%02x", *ptin++);
+                ptout += 3;
+                break;
+        }
     }
-    /* printf ("%lu bytes, all ASCII\n", (unsigned long)(s - data->bv_val)); */
-    return data->bv_val;
+    *ptout = 0;
+    return buf;
 }
 
 static const char *