From e0cca3fcab80fe3f25f79bb39c631b229b5d6e8a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Kuzn=C3=ADk?= <ondra@mistotebe.net>
Date: Wed, 8 Oct 2025 17:10:06 +0100
Subject: [PATCH] ITS#10313 Tighten counter tracking modification

Try to make sure the counter/timer value we used hasn't been used up in
the meantime. Also if the update cannot be committed, do not say whether
the provided OTP was correct, this would open up an oracle for malicious
clients to brute force a token they could use later/elsewhere.
---
 servers/slapd/overlays/otp.c | 43 ++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/servers/slapd/overlays/otp.c b/servers/slapd/overlays/otp.c
index e2ffab03e8..772c9d2d98 100644
--- a/servers/slapd/overlays/otp.c
+++ b/servers/slapd/overlays/otp.c
@@ -649,7 +649,7 @@ otp_bind_response( Operation *op, SlapReply *rs )
 }
 
 static long
-otp_hotp( Operation *op, Entry *token )
+otp_hotp( Operation *op, Entry *token, BerValue *old_value )
 {
 	char outbuf[MAX_DIGITS + 1];
 	Entry *params = NULL;
@@ -669,6 +669,9 @@ otp_hotp( Operation *op, Entry *token )
 				a->a_vals[0].bv_val );
 		goto done;
 	}
+	if ( a ) {
+		ber_bvreplace( old_value, &a->a_vals[0] );
+	}
 
 	a = attr_find( token->e_attrs, ad_oathHOTPParams );
 	if ( !a ||
@@ -753,7 +756,7 @@ done:
 }
 
 static long
-otp_totp( Operation *op, Entry *token, long *drift )
+otp_totp( Operation *op, Entry *token, BerValue *old_value, long *drift )
 {
 	char outbuf[MAX_DIGITS + 1];
 	Entry *params = NULL;
@@ -773,6 +776,9 @@ otp_totp( Operation *op, Entry *token, long *drift )
 				a->a_vals[0].bv_val );
 		goto done;
 	}
+	if ( a ) {
+		ber_bvreplace( old_value, &a->a_vals[0] );
+	}
 
 	a = attr_find( token->e_attrs, ad_oathTOTPParams );
 	if ( !a ||
@@ -885,7 +891,7 @@ otp_check_and_update( Operation *op, BerValue *totpdn, BerValue *hotpdn,
 		Entry *token )
 {
 	AttributeDescription *ad = NULL, *drift_ad = NULL;
-	BerValue ndn;
+	BerValue ndn, old_value[2] = { BER_BVNULL, BER_BVNULL };
 	long t = -1, drift = 0;
 	int rc = LDAP_INVALID_CREDENTIALS;
 
@@ -895,7 +901,7 @@ otp_check_and_update( Operation *op, BerValue *totpdn, BerValue *hotpdn,
 			ndn = *totpdn;
 			ad = ad_oathTOTPLastTimeStep;
 			drift_ad = ad_oathTOTPTimeStepDrift;
-			t = otp_totp( op, token, &drift );
+			t = otp_totp( op, token, &old_value[0], &drift );
 			be_entry_release_r( op, token );
 			token = NULL;
 		}
@@ -905,7 +911,7 @@ otp_check_and_update( Operation *op, BerValue *totpdn, BerValue *hotpdn,
 					0, &token ) == LDAP_SUCCESS ) {
 			ndn = *hotpdn;
 			ad = ad_oathHOTPCounter;
-			t = otp_hotp( op, token );
+			t = otp_hotp( op, token, &old_value[0] );
 			be_entry_release_r( op, token );
 			token = NULL;
 		}
@@ -917,7 +923,7 @@ otp_check_and_update( Operation *op, BerValue *totpdn, BerValue *hotpdn,
 		char outbuf[32], drift_buf[32];
 		Operation op2;
 		Opheader oh;
-		Modifications mod[2], *m = mod;
+		Modifications mod[3], *m = mod;
 		SlapReply rs2 = { REP_RESULT };
 		slap_callback cb = { .sc_response = &slap_null_cb };
 		BerValue bv[2], bv_drift[2];
@@ -926,15 +932,27 @@ otp_check_and_update( Operation *op, BerValue *totpdn, BerValue *hotpdn,
 		bv[0].bv_len = snprintf( bv[0].bv_val, sizeof(outbuf), "%ld", t );
 		BER_BVZERO( &bv[1] );
 
+		/* Limit races by removing old counter by value */
+		if ( !BER_BVISNULL( &old_value[0] ) ) {
+			m->sml_numvals = 1;
+			m->sml_values = old_value;
+			m->sml_nvalues = NULL;
+			m->sml_desc = ad;
+			m->sml_op = LDAP_MOD_DELETE;
+			m->sml_flags = SLAP_MOD_INTERNAL;
+			m->sml_next = (m+1);
+			m++;
+		}
+
 		m->sml_numvals = 1;
 		m->sml_values = bv;
 		m->sml_nvalues = NULL;
 		m->sml_desc = ad;
-		m->sml_op = LDAP_MOD_REPLACE;
+		m->sml_op = LDAP_MOD_ADD;
 		m->sml_flags = SLAP_MOD_INTERNAL;
 
 		if ( drift_ad ) {
-			m->sml_next = &mod[1];
+			m->sml_next = (m+1);
 
 			bv_drift[0].bv_val = drift_buf;
 			bv_drift[0].bv_len = snprintf(
@@ -971,13 +989,20 @@ otp_check_and_update( Operation *op, BerValue *totpdn, BerValue *hotpdn,
 
 		rc = op2.o_bd->be_modify( &op2, &rs2 );
 		if ( rs2.sr_err != LDAP_SUCCESS ) {
-			rc = LDAP_OTHER;
+			Debug( LDAP_DEBUG_ANY, "%s otp_check_and_update: "
+				"failed to invalidate provided token rc=%d: %s\n",
+				op->o_log_prefix, rc, rs2.sr_text ? rs2.sr_text : "" );
+			rc = LDAP_INVALID_CREDENTIALS;
 		}
 		if ( m->sml_next ) {
 			slap_mods_free( m->sml_next, 1 );
 		}
 	}
 
+	if ( !BER_BVISNULL( &old_value[0] ) ) {
+		ch_free( old_value[0].bv_val );
+	}
+
 	return rc;
 }