#364 Add test for bogus hashes

- Verify that a "hash" in the wrong format doesn't throw exception (this is relevant when the supportOldPasswordHash setting is enabled)

src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java

@@ -13,11 +13,13 @@
 // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 package fr.xephi.authme.security.crypts;
 
+import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.security.crypts.description.HasSalt;
 import fr.xephi.authme.security.crypts.description.Usage;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.SaltType;
 import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.util.StringUtils;
 
 import java.io.UnsupportedEncodingException;
 import java.security.SecureRandom;
@@ -527,7 +529,12 @@ public class BCRYPT implements EncryptionMethod {
 
     @Override
     public boolean comparePassword(String password, HashedPassword hash, String name) {
-        return checkpw(password, hash.getHash());
+        try {
+            return hash.getHash().length() > 3 && checkpw(password, hash.getHash());
+        } catch (IllegalArgumentException e) {
+            ConsoleLogger.showError("Bcrypt checkpw() returned " + StringUtils.formatException(e));
+        }
+        return false;
     }
 
     @Override

src/main/java/fr/xephi/authme/security/crypts/CryptPBKDF2.java

@@ -22,6 +22,9 @@ public class CryptPBKDF2 extends HexSaltedMethod {
     @Override
     public boolean comparePassword(String password, HashedPassword hashedPassword, String unusedName) {
         String[] line = hashedPassword.getHash().split("\\$");
+        if (line.length != 4) {
+            return false;
+        }
         String salt = line[2];
         String derivedKey = line[3];
         PBKDF2Parameters params = new PBKDF2Parameters("HmacSHA256", "ASCII", salt.getBytes(), 10000, derivedKey.getBytes());

src/main/java/fr/xephi/authme/security/crypts/CryptPBKDF2Django.java

@@ -21,6 +21,9 @@ public class CryptPBKDF2Django extends HexSaltedMethod {
     @Override
     public boolean comparePassword(String password, HashedPassword hashedPassword, String unusedName) {
         String[] line = hashedPassword.getHash().split("\\$");
+        if (line.length != 4) {
+            return false;
+        }
         String salt = line[2];
         byte[] derivedKey = DatatypeConverter.parseBase64Binary(line[3]);
         PBKDF2Parameters params = new PBKDF2Parameters("HmacSHA256", "ASCII", salt.getBytes(), 15000, derivedKey);

src/main/java/fr/xephi/authme/security/crypts/WHIRLPOOL.java

@@ -379,6 +379,7 @@ public class WHIRLPOOL extends UnsaltedMethod {
         }
     }
 
+    @Override
     public String computeHash(String password) {
         byte[] digest = new byte[DIGESTBYTES];
         NESSIEinit();

src/main/java/fr/xephi/authme/security/crypts/XAUTH.java

@@ -26,8 +26,11 @@ public class XAUTH extends HexSaltedMethod {
     public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) {
         String hash = hashedPassword.getHash();
         int saltPos = (password.length() >= hash.length() ? hash.length() - 1 : password.length());
-        String saltFromHash = hash.substring(saltPos, saltPos + 12);
-        return hash.equals(computeHash(password, saltFromHash, null));
+        if (saltPos + 12 > hash.length()) {
+            return false;
+        }
+        String salt = hash.substring(saltPos, saltPos + 12);
+        return hash.equals(computeHash(password, salt, null));
     }
 
     @Override

src/test/java/fr/xephi/authme/security/crypts/AbstractEncryptionMethodTest.java

@@ -32,6 +32,10 @@ public abstract class AbstractEncryptionMethodTest {
     private static final List<String> INTERNAL_PASSWORDS =
         ImmutableList.of("test1234", "Ab_C73", "(!#&$~`_-Aa0", "Ûïé1&?+A");
 
+    private static final String[] BOGUS_HASHES = {"", "test", "$t$test$", "$SHA$Test$$$$", "$$$$$",
+        "asdfg:hjkl", "::test", "~#$#~~~#$#~", "d41d8cd98f00b204e9800998ecf427e",
+        "$2y$7a$da641e404b982ed" };
+
     /** The encryption method to test. */
     private EncryptionMethod method;
     /** Map with the hashes against which the entries in GIVEN_PASSWORDS are tested. */
@@ -123,6 +127,17 @@ public abstract class AbstractEncryptionMethodTest {
         }
     }
 
+    /** Tests various strings to ensure that encryption methods don't rely on the hash's format too much. */
+    @Test
+    public void testMalformedHashes() {
+        String salt = method.hasSeparateSalt() ? "testSalt" : null;
+        for (String bogusHash : BOGUS_HASHES) {
+            HashedPassword hashedPwd = new HashedPassword(bogusHash, salt);
+            assertFalse("Passing bogus hash '" + bogusHash + "' does not result in an error",
+                method.comparePassword("Password", hashedPwd, "player"));
+        }
+    }
+
     private boolean doesGivenHashMatch(String password, EncryptionMethod method) {
         return method.comparePassword(password, hashes.get(password), USERNAME);
     }