Skip to content

Commit 3760d79

Browse files
committed
SEC-1890: Add checks for validity of stored bcrypt hash
When checking for a match, the BCryptPasswordEncoder validates the stored hash against a pattern to check that it actually is a bcrypt value.
1 parent 5d71d2a commit 3760d79

File tree

2 files changed

+48
-5
lines changed

2 files changed

+48
-5
lines changed

crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.security.crypto.bcrypt;
1717

1818
import java.security.SecureRandom;
19+
import java.util.regex.Pattern;
1920

2021
import org.springframework.security.crypto.password.PasswordEncoder;
2122

@@ -28,6 +29,7 @@
2829
*
2930
*/
3031
public class BCryptPasswordEncoder implements PasswordEncoder {
32+
private Pattern BCRYPT_PATTERN = Pattern.compile("\\A\\$2a?\\$\\d\\d\\$[./0-9A-Za-z]{53}");
3133

3234
private final int strength;
3335

@@ -71,7 +73,14 @@ public String encode(CharSequence rawPassword) {
7173
}
7274

7375
public boolean matches(CharSequence rawPassword, String encodedPassword) {
76+
if (encodedPassword == null || encodedPassword.length() == 0) {
77+
throw new IllegalArgumentException("Encoded password cannot be null or empty");
78+
}
79+
80+
if (!BCRYPT_PATTERN.matcher(encodedPassword).matches()) {
81+
throw new IllegalArgumentException("Encoded password does not look like BCrypt");
82+
}
83+
7484
return BCrypt.checkpw(rawPassword.toString(), encodedPassword);
7585
}
76-
7786
}

crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
import org.junit.Test;
2222

23+
import java.security.SecureRandom;
24+
2325

2426
/**
2527
* @author Dave Syer
@@ -44,17 +46,49 @@ public void unicode() {
4446
}
4547

4648
@Test
47-
public void matchesLengthChecked() {
49+
public void notMatches() {
4850
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
4951
String result = encoder.encode("password");
50-
assertFalse(encoder.matches("password", result.substring(0,result.length()-2)));
52+
assertFalse(encoder.matches("bogus", result));
5153
}
5254

5355
@Test
54-
public void notMatches() {
56+
public void customStrength() {
57+
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(8);
58+
String result = encoder.encode("password");
59+
assertTrue(encoder.matches("password", result));
60+
}
61+
62+
@Test
63+
public void customRandom() {
64+
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(8, new SecureRandom());
65+
String result = encoder.encode("password");
66+
assertTrue(encoder.matches("password", result));
67+
}
68+
69+
@Test(expected = IllegalArgumentException.class)
70+
public void barfsOnNullEncodedValue() {
71+
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
72+
assertFalse(encoder.matches("password", null));
73+
}
74+
75+
@Test(expected = IllegalArgumentException.class)
76+
public void barfsOnEmptyEncodedValue() {
77+
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
78+
assertFalse(encoder.matches("password", ""));
79+
}
80+
81+
@Test(expected = IllegalArgumentException.class)
82+
public void barfsOnShortEncodedValue() {
5583
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
5684
String result = encoder.encode("password");
57-
assertFalse(encoder.matches("bogus", result));
85+
assertFalse(encoder.matches("password", result.substring(0, 4)));
86+
}
87+
88+
@Test(expected = IllegalArgumentException.class)
89+
public void barfsOnBogusEncodedValue() {
90+
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
91+
assertFalse(encoder.matches("password", "012345678901234567890123456789"));
5892
}
5993

6094
}

0 commit comments

Comments
 (0)