SEC-1031: LdapShaPasswordEncoder.isPasswordValid startOfHash off by one

http://jira.springframework.org/browse/SEC-1031. Fixed startOfHash value and added tests to check full length of password is used.

core/src/main/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoder.java

@@ -86,9 +86,9 @@ public class LdapShaPasswordEncoder implements PasswordEncoder {
             sha.update(rawPass.getBytes("UTF-8"));
         } catch (java.security.NoSuchAlgorithmException e) {
             throw new IllegalStateException("No SHA implementation available!");
-		} catch (UnsupportedEncodingException ue) {
-			throw new IllegalStateException("UTF-8 not supported!");
-		}
+        } catch (UnsupportedEncodingException ue) {
+            throw new IllegalStateException("UTF-8 not supported!");
+        }
 
         if (salt != null) {
             Assert.isInstanceOf(byte[].class, salt, "Salt value must be a byte array");
@@ -131,7 +131,7 @@ public class LdapShaPasswordEncoder implements PasswordEncoder {
      */
     public boolean isPasswordValid(final String encPass, final String rawPass, Object salt) {
         String prefix = extractPrefix(encPass);
-        
+
         if (prefix == null) {
             return encPass.equals(rawPass);
         }
@@ -141,32 +141,32 @@ public class LdapShaPasswordEncoder implements PasswordEncoder {
         } else if (!prefix.equals(SHA_PREFIX) && !prefix.equals(SHA_PREFIX_LC)) {
             throw new IllegalArgumentException("Unsupported password prefix '" + prefix + "'");
         } else {
-        	// Standard SHA
-        	salt = null;
+            // Standard SHA
+            salt = null;
         }
 
-        int startOfHash = prefix.length() + 1;
-        
+        int startOfHash = prefix.length();
+
         String encodedRawPass = encodePassword(rawPass, salt).substring(startOfHash);
-        
+
         return encodedRawPass.equals(encPass.substring(startOfHash));
     }
-    
+
     /**
-     * Returns the hash prefix or null if there isn't one. 
+     * Returns the hash prefix or null if there isn't one.
      */
     private String extractPrefix(String encPass) {
         if (!encPass.startsWith("{")) {
-        	return null;
+            return null;
+        }
+
+        int secondBrace = encPass.lastIndexOf('}');
+
+        if (secondBrace < 0) {
+            throw new IllegalArgumentException("Couldn't find closing brace for SHA prefix");
         }
 
-		int secondBrace = encPass.lastIndexOf('}');
-		
-		if (secondBrace < 0) {
-			throw new IllegalArgumentException("Couldn't find closing brace for SHA prefix");
-		}
-		
-		return encPass.substring(0, secondBrace + 1);
+        return encPass.substring(0, secondBrace + 1);
     }
 
     public void setForceLowerCasePrefix(boolean forceLowerCasePrefix) {

core/src/test/java/org/springframework/security/providers/ldap/authenticator/LdapShaPasswordEncoderTests.java

@@ -44,7 +44,7 @@ public class LdapShaPasswordEncoderTests {
         assertFalse(sha.isPasswordValid("{SHA}ddSFGmjXYPbZC+NXR2kCzBRjqiE=", "wrongpassword", null));
     }
 
-    @Test    
+    @Test
     public void invalidSaltedPasswordFails() {
         assertFalse(sha.isPasswordValid("{SSHA}25ro4PKC8jhQZ26jVsozhX/xaP0suHgX", "wrongpassword", null));
         assertFalse(sha.isPasswordValid("{SSHA}PQy2j+6n5ytA+YlAKkM8Fh4p6u2JxfVd", "wrongpassword", null));
@@ -81,6 +81,15 @@ public class LdapShaPasswordEncoderTests {
         assertTrue(sha.isPasswordValid("{ssha}PQy2j+6n5ytA+YlAKkM8Fh4p6u2JxfVd", "boabspasswurd", null));
     }
 
+    @Test
+    // SEC-1031
+    public void fullLengthOfHashIsUsedInComparison() throws Exception {
+        // Change the first hash character from '2' to '3'
+        assertFalse(sha.isPasswordValid("{SSHA}35ro4PKC8jhQZ26jVsozhX/xaP0suHgX", "boabspasswurd", null));
+     // Change the last hash character from 'X' to 'Y'
+        assertFalse(sha.isPasswordValid("{SSHA}25ro4PKC8jhQZ26jVsozhX/xaP0suHgY", "boabspasswurd", null));
+    }
+
     @Test
     public void correctPrefixCaseIsUsed() {
         sha.setForceLowerCasePrefix(false);
@@ -95,12 +104,12 @@ public class LdapShaPasswordEncoderTests {
 
     @Test(expected=IllegalArgumentException.class)
     public void invalidPrefixIsRejected() {
-    	sha.isPasswordValid("{MD9}xxxxxxxxxx" , "somepassword", null);
+        sha.isPasswordValid("{MD9}xxxxxxxxxx" , "somepassword", null);
     }
-    
+
     @Test(expected=IllegalArgumentException.class)
     public void malformedPrefixIsRejected() {
-    	// No right brace
-    	sha.isPasswordValid("{SSHA25ro4PKC8jhQZ26jVsozhX/xaP0suHgX" , "somepassword", null);
+        // No right brace
+        sha.isPasswordValid("{SSHA25ro4PKC8jhQZ26jVsozhX/xaP0suHgX" , "somepassword", null);
     }
 }