spring-security
spring-security copied to clipboard
HaveIBeenPwnedRestApiPasswordChecker implementations are not thread-safe
HaveIBeenPwnedRestApiPasswordChecker stores a single MessageDigest instance as a field and reuses it across all invocations of check(). Since MessageDigest is not thread-safe, concurrent calls can produce incorrect hash values.
To Reproduce
- Register
HaveIBeenPwnedRestApiPasswordCheckeras a singleton bean - Call
check()concurrently from multiple threads - Hash computation becomes unreliable due to shared mutable state in
MessageDigest
Expected behavior
The checker should produce correct results under concurrent access. A new MessageDigest instance should be created per invocation instead of reusing a shared instance.
Hi Team,
Added a small reproducible demo:
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.HexFormat;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
public class MessageDigestThreadSafetyDemo {
// Shared MessageDigest instance (NOT thread-safe!)
private static final MessageDigest SHARED_MD;
static {
try {
SHARED_MD = MessageDigest.getInstance("SHA-1");
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException(e);
}
}
static void main(String[] args) throws Exception {
String input = "Hello World";
String expectedHash = computeHashSafely(input);
System.out.println("Expected hash: " + expectedHash);
int threadCount = 10;
int iterationsPerThread = 1000;
CountDownLatch latch = new CountDownLatch(threadCount);
AtomicInteger failures = new AtomicInteger(0);
for (int t = 0; t < threadCount; t++) {
Thread.startVirtualThread(() -> {
try {
for (int i = 0; i < iterationsPerThread; i++) {
String hash = computeHashUnsafely(input);
if (!expectedHash.equals(hash)) {
System.out.println("wrong hash: " + hash);
failures.incrementAndGet();
}
}
} finally {
latch.countDown();
}
});
}
latch.await();
System.out.println("Total operations: " + (threadCount * iterationsPerThread));
System.out.println("Failures (wrong hash): " + failures.get());
}
// UNSAFE: Uses shared MessageDigest instance
private static String computeHashUnsafely(String input) {
byte[] hash = SHARED_MD.digest(input.getBytes());
return HexFormat.of().formatHex(hash);
}
// SAFE: Creates new instance each time
private static String computeHashSafely(String input) throws NoSuchAlgorithmException {
MessageDigest md = MessageDigest.getInstance("SHA-1");
byte[] hash = md.digest(input.getBytes());
return HexFormat.of().formatHex(hash);
}
}
outputs:
Expected hash: 0a4d55a8d778e5022fab701977c5d840bbc486d0
wrong hash: 5dc0012ad54717e3c566b0a84c7085693dba31bd
wrong hash: b38bee8defe04cfba4c5d7f032b6a7d2e4afa520
wrong hash: da39a3ee5e6b4b0d3255bfef95601890afd80709
wrong hash: 16a816dad565af32c5618209370704742658e04f
.
.
.
wrong hash: da39a3ee5e6b4b0d3255bfef95601890afd80709
Total operations: 10000
Failures (wrong hash): 400