From 36a2f1d5e4e2265140320087104c6799a97c28d9 Mon Sep 17 00:00:00 2001
From: James Chapman <jachapma@redhat.com>
Date: Wed, 1 May 2024 15:01:33 +0100
Subject: [PATCH] Security fix for CVE-2024-2199

Description:
A denial of service vulnerability was found in the 389 Directory Server.
This issue may allow an authenticated user to cause a server crash while
modifying userPassword using malformed input.

Fix Description:
When doing a mod on userPassword we reset the pblock modifier after we
set the modified timestamp, ensuring the pblock data stays valid.

References:
- https://nvd.nist.gov/vuln/detail/CVE-2024-2199
- https://access.redhat.com/security/cve/CVE-2024-2199
- https://bugzilla.redhat.com/show_bug.cgi?id=2267976
---
 .../tests/suites/password/password_test.py    | 56 +++++++++++++++++++
 ldap/servers/slapd/modify.c                   |  8 ++-
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/dirsrvtests/tests/suites/password/password_test.py b/dirsrvtests/tests/suites/password/password_test.py
index 1245feb31..e4abd9907 100644
--- a/dirsrvtests/tests/suites/password/password_test.py
+++ b/dirsrvtests/tests/suites/password/password_test.py
@@ -63,6 +63,62 @@ def test_password_delete_specific_password(topology_st):
     log.info('test_password_delete_specific_password: PASSED')
 
 
+def test_password_modify_non_utf8(topology_st):
+    """Attempt a modify of the userPassword attribute with
+    an invalid non utf8 value
+
+    :id: a31af9d5-d665-42b9-8d6e-fea3d0837d36
+    :setup: Standalone instance
+    :steps:
+        1. Add a user if it doesnt exist and set its password
+        2. Verify password with a bind
+        3. Modify userPassword attr with invalid value
+        4. Attempt a bind with invalid password value
+        5. Verify original password with a bind
+    :expectedresults:
+        1. The user with userPassword should be added successfully
+        2. Operation should be successful
+        3. Server returns ldap.UNWILLING_TO_PERFORM
+        4. Server returns ldap.INVALID_CREDENTIALS
+        5. Operation should be successful
+     """
+
+    log.info('Running test_password_modify_non_utf8...')
+
+    # Create user and set password
+    standalone = topology_st.standalone
+    users = UserAccounts(standalone, DEFAULT_SUFFIX)
+    if not users.exists(TEST_USER_PROPERTIES['uid'][0]):
+        user = users.create(properties=TEST_USER_PROPERTIES)
+    else:
+        user = users.get(TEST_USER_PROPERTIES['uid'][0])
+    user.set('userpassword', PASSWORD)
+
+    # Verify password
+    try:
+        user.bind(PASSWORD)
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as {}, error: '.format(user.dn) + e.args[0]['desc'])
+        assert False
+
+    # Modify userPassword with an invalid value
+    password = b'tes\x82t-password' # A non UTF-8 encoded password
+    with pytest.raises(ldap.UNWILLING_TO_PERFORM):
+        user.replace('userpassword', password)
+
+    # Verify a bind fails with invalid pasword
+    with pytest.raises(ldap.INVALID_CREDENTIALS):
+        user.bind(password)
+
+    # Verify we can still bind with original password
+    try:
+        user.bind(PASSWORD)
+    except ldap.LDAPError as e:
+        log.fatal('Failed to bind as {}, error: '.format(user.dn) + e.args[0]['desc'])
+        assert False
+
+    log.info('test_password_modify_non_utf8: PASSED')
+
 if __name__ == '__main__':
     # Run isolated
     # -s for DEBUG mode
diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c
index a20984e0b..fb65d58b3 100644
--- a/ldap/servers/slapd/modify.c
+++ b/ldap/servers/slapd/modify.c
@@ -762,8 +762,10 @@ op_shared_modify(Slapi_PBlock *pb, int pw_change, char *old_pw)
      * flagged - leave mod attributes alone */
     if (!repl_op && !skip_modified_attrs && lastmod) {
         modify_update_last_modified_attr(pb, &smods);
+        slapi_pblock_set(pb, SLAPI_MODIFY_MODS, slapi_mods_get_ldapmods_byref(&smods));
     }
 
+
     if (0 == slapi_mods_get_num_mods(&smods)) {
         /* nothing to do - no mods - this is not an error - just
            send back LDAP_SUCCESS */
@@ -930,8 +932,10 @@ op_shared_modify(Slapi_PBlock *pb, int pw_change, char *old_pw)
 
             /* encode password */
             if (pw_encodevals_ext(pb, sdn, va)) {
-                slapi_log_err(SLAPI_LOG_CRIT, "op_shared_modify", "Unable to hash userPassword attribute for %s.\n", slapi_entry_get_dn_const(e));
-                send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM, NULL, "Unable to store attribute \"userPassword\" correctly\n", 0, NULL);
+                slapi_log_err(SLAPI_LOG_CRIT, "op_shared_modify", "Unable to hash userPassword attribute for %s, "
+                    "check value is utf8 string.\n", slapi_entry_get_dn_const(e));
+                send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM, NULL, "Unable to hash \"userPassword\" attribute, "
+                    "check value is utf8 string.\n", 0, NULL);
                 valuearray_free(&va);
                 goto free_and_return;
             }
-- 
2.35.3

