From 3760d792eacaa089c6ab2426801fabd5e80574d9 Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Wed, 22 Feb 2012 14:36:13 +0000
Subject: [PATCH] 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.
---
 .../crypto/bcrypt/BCryptPasswordEncoder.java  | 11 ++++-
 .../bcrypt/BCryptPasswordEncoderTests.java    | 42 +++++++++++++++++--
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java b/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java
index 697e29c42d..e416e9eb01 100644
--- a/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java
+++ b/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java
@@ -16,6 +16,7 @@
 package org.springframework.security.crypto.bcrypt;
 
 import java.security.SecureRandom;
+import java.util.regex.Pattern;
 
 import org.springframework.security.crypto.password.PasswordEncoder;
 
@@ -28,6 +29,7 @@ import org.springframework.security.crypto.password.PasswordEncoder;
  *
  */
 public class BCryptPasswordEncoder implements PasswordEncoder {
+    private Pattern BCRYPT_PATTERN = Pattern.compile("\\A\\$2a?\\$\\d\\d\\$[./0-9A-Za-z]{53}");
 
     private final int strength;
 
@@ -71,7 +73,14 @@ public class BCryptPasswordEncoder implements PasswordEncoder {
     }
 
     public boolean matches(CharSequence rawPassword, String encodedPassword) {
+        if (encodedPassword == null || encodedPassword.length() == 0) {
+            throw new IllegalArgumentException("Encoded password cannot be null or empty");
+        }
+
+        if (!BCRYPT_PATTERN.matcher(encodedPassword).matches()) {
+            throw new IllegalArgumentException("Encoded password does not look like BCrypt");
+        }
+
         return BCrypt.checkpw(rawPassword.toString(), encodedPassword);
     }
-
 }
diff --git a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java
index df341cb861..69f4d47b51 100644
--- a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java
+++ b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java
@@ -20,6 +20,8 @@ import static org.junit.Assert.assertTrue;
 
 import org.junit.Test;
 
+import java.security.SecureRandom;
+
 
 /**
  * @author Dave Syer
@@ -44,17 +46,49 @@ public class BCryptPasswordEncoderTests {
     }
 
     @Test
-    public void matchesLengthChecked() {
+    public void notMatches() {
         BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
         String result = encoder.encode("password");
-        assertFalse(encoder.matches("password", result.substring(0,result.length()-2)));
+        assertFalse(encoder.matches("bogus", result));
     }
 
     @Test
-    public void notMatches() {
+    public void customStrength() {
+        BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(8);
+        String result = encoder.encode("password");
+        assertTrue(encoder.matches("password", result));
+    }
+
+    @Test
+    public void customRandom() {
+        BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(8, new SecureRandom());
+        String result = encoder.encode("password");
+        assertTrue(encoder.matches("password", result));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void barfsOnNullEncodedValue() {
+        BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
+        assertFalse(encoder.matches("password", null));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void barfsOnEmptyEncodedValue() {
+        BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
+        assertFalse(encoder.matches("password", ""));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void barfsOnShortEncodedValue() {
         BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
         String result = encoder.encode("password");
-        assertFalse(encoder.matches("bogus", result));
+        assertFalse(encoder.matches("password", result.substring(0, 4)));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void barfsOnBogusEncodedValue() {
+        BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
+        assertFalse(encoder.matches("password", "012345678901234567890123456789"));
     }
 
 }