From 52a9ee6556a0467f5134fb6392ff1681a38f3252 Mon Sep 17 00:00:00 2001 From: Pierre Rogier Date: Fri, 14 Jun 2024 13:27:10 +0200 Subject: [PATCH] CVE-2024-5953 --- .../tests/suites/password/regression_test.py | 51 ++++++++++++++++++- ldap/servers/plugins/pwdstorage/md5_pwd.c | 9 +++- ldap/servers/plugins/pwdstorage/pbkdf2_pwd.c | 6 +++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/dirsrvtests/tests/suites/password/regression_test.py b/dirsrvtests/tests/suites/password/regression_test.py index 4876ff435..160d6f01d 100644 --- a/dirsrvtests/tests/suites/password/regression_test.py +++ b/dirsrvtests/tests/suites/password/regression_test.py @@ -8,11 +8,12 @@ import pytest import time import glob +import base64 from lib389._constants import PASSWORD, DN_DM, DEFAULT_SUFFIX from lib389._constants import SUFFIX, PASSWORD, DN_DM, DN_CONFIG, PLUGIN_RETRO_CHANGELOG, DEFAULT_SUFFIX, DEFAULT_CHANGELOG_DB, DEFAULT_BENAME from lib389 import Entry from lib389.topologies import topology_m1 as topo_supplier -from lib389.idm.user import UserAccounts +from lib389.idm.user import UserAccounts, UserAccount from lib389.utils import ldap, os, logging, ensure_bytes, ds_is_newer, ds_supports_new_changelog from lib389.topologies import topology_st as topo from lib389.idm.organizationalunit import OrganizationalUnits @@ -40,6 +41,13 @@ TEST_PASSWORDS += ['CNpwtest1ZZZZ', 'ZZZZZCNpwtest1', TEST_PASSWORDS2 = ( 'CN12pwtest31', 'SN3pwtest231', 'UID1pwtest123', 'MAIL2pwtest12@redhat.com', '2GN1pwtest123', 'People123') +SUPPORTED_SCHEMES = ( + "{SHA}", "{SSHA}", "{SHA256}", "{SSHA256}", + "{SHA384}", "{SSHA384}", "{SHA512}", "{SSHA512}", + "{crypt}", "{NS-MTA-MD5}", "{clear}", "{MD5}", + "{SMD5}", "{PBKDF2_SHA256}", "{PBKDF2_SHA512}", + "{GOST_YESCRYPT}", "{PBKDF2-SHA256}", "{PBKDF2-SHA512}" ) + def _check_unhashed_userpw(inst, user_dn, is_present=False): """Check if unhashed#user#password attribute is present or not in the changelog""" unhashed_pwd_attribute = 'unhashed#user#password' @@ -319,6 +327,47 @@ def test_unhashed_pw_switch(topo_supplier): # Add debugging steps(if any)... pass +@pytest.mark.parametrize("scheme", SUPPORTED_SCHEMES ) +def test_long_hashed_password(topo, create_user, scheme): + """Check that hashed password with very long value does not cause trouble + + :id: 252a1f76-114b-11ef-8a7a-482ae39447e5 + :setup: standalone Instance + :parametrized: yes + :steps: + 1. Add a test user user + 2. Set a long password with requested scheme + 3. Bind on that user using a wrong password + 4. Check that instance is still alive + 5. Remove the added user + :expectedresults: + 1. Success + 2. Success + 3. Should get ldap.INVALID_CREDENTIALS exception + 4. Success + 5. Success + """ + inst = topo.standalone + inst.simple_bind_s(DN_DM, PASSWORD) + users = UserAccounts(inst, DEFAULT_SUFFIX) + # Make sure that server is started as this test may crash it + inst.start() + # Adding Test user (It may already exists if previous test failed) + user2 = UserAccount(inst, dn='uid=test_user_1002,ou=People,dc=example,dc=com') + if not user2.exists(): + user2 = users.create_test_user(uid=1002, gid=2002) + # Setting hashed password + passwd = 'A'*4000 + hashed_passwd = scheme.encode('utf-8') + base64.b64encode(passwd.encode('utf-8')) + user2.replace('userpassword', hashed_passwd) + # Bind on that user using a wrong password + with pytest.raises(ldap.INVALID_CREDENTIALS): + conn = user2.bind(PASSWORD) + # Check that instance is still alive + assert inst.status() + # Remove the added user + user2.delete() + if __name__ == '__main__': # Run isolated diff --git a/ldap/servers/plugins/pwdstorage/md5_pwd.c b/ldap/servers/plugins/pwdstorage/md5_pwd.c index 1e2cf58e7..b9a48d5ca 100644 --- a/ldap/servers/plugins/pwdstorage/md5_pwd.c +++ b/ldap/servers/plugins/pwdstorage/md5_pwd.c @@ -37,6 +37,7 @@ md5_pw_cmp(const char *userpwd, const char *dbpwd) unsigned char hash_out[MD5_HASH_LEN]; unsigned char b2a_out[MD5_HASH_LEN * 2]; /* conservative */ SECItem binary_item; + size_t dbpwd_len = strlen(dbpwd); ctx = PK11_CreateDigestContext(SEC_OID_MD5); if (ctx == NULL) { @@ -45,6 +46,12 @@ md5_pw_cmp(const char *userpwd, const char *dbpwd) goto loser; } + if (dbpwd_len >= sizeof b2a_out) { + slapi_log_err(SLAPI_LOG_PLUGIN, MD5_SUBSYSTEM_NAME, + "The hashed password stored in the user entry is longer than any valid md5 hash"); + goto loser; + } + /* create the hash */ PK11_DigestBegin(ctx); PK11_DigestOp(ctx, (const unsigned char *)userpwd, strlen(userpwd)); @@ -57,7 +64,7 @@ md5_pw_cmp(const char *userpwd, const char *dbpwd) bver = NSSBase64_EncodeItem(NULL, (char *)b2a_out, sizeof b2a_out, &binary_item); /* bver points to b2a_out upon success */ if (bver) { - rc = slapi_ct_memcmp(bver, dbpwd, strlen(dbpwd)); + rc = slapi_ct_memcmp(bver, dbpwd, dbpwd_len); } else { slapi_log_err(SLAPI_LOG_PLUGIN, MD5_SUBSYSTEM_NAME, "Could not base64 encode hashed value for password compare"); diff --git a/ldap/servers/plugins/pwdstorage/pbkdf2_pwd.c b/ldap/servers/plugins/pwdstorage/pbkdf2_pwd.c index dcac4fcdd..82b8c9501 100644 --- a/ldap/servers/plugins/pwdstorage/pbkdf2_pwd.c +++ b/ldap/servers/plugins/pwdstorage/pbkdf2_pwd.c @@ -255,6 +255,12 @@ pbkdf2_sha256_pw_cmp(const char *userpwd, const char *dbpwd) passItem.data = (unsigned char *)userpwd; passItem.len = strlen(userpwd); + if (pwdstorage_base64_decode_len(dbpwd, dbpwd_len) > sizeof dbhash) { + /* Hashed value is too long and cannot match any value generated by pbkdf2_sha256_hash */ + slapi_log_err(SLAPI_LOG_ERR, (char *)schemeName, "Unable to base64 decode dbpwd value. (hashed value is too long)\n"); + return result; + } + /* Decode the DBpwd to bytes from b64 */ if (PL_Base64Decode(dbpwd, dbpwd_len, dbhash) == NULL) { slapi_log_err(SLAPI_LOG_ERR, (char *)schemeName, "Unable to base64 decode dbpwd value\n"); -- 2.44.0