Md5Crypt now throws IllegalArgumentException on an invalid prefix

garydgregory · garydgregory · commit ad28b0f1ea40 · 2024-05-16T16:44:32.000-04:00
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -45,6 +45,7 @@ The <action> type attribute can be add,update,fix,remove.
   <body>
     <release version="1.17.1" date="YYYY-MM-DD" description="Feature and fix release. Requires a minimum of Java 8.">
       <!-- FIX -->
+      <action type="fix" dev="ggregory" due-to="Gary Gregory">Md5Crypt now throws IllegalArgumentException on an invalid prefix.</action>
       <!-- ADD -->
       <!-- UPDATE -->
     </release>
diff --git a/src/main/java/org/apache/commons/codec/digest/Md5Crypt.java b/src/main/java/org/apache/commons/codec/digest/Md5Crypt.java
@@ -20,6 +20,7 @@
 import java.security.MessageDigest;
 import java.security.SecureRandom;
 import java.util.Arrays;
+import java.util.Objects;
 import java.util.Random;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -238,7 +239,7 @@ public static String md5Crypt(final byte[] keyBytes, final String salt) {
      *            real salt value without prefix or "rounds=". The salt may be null, in which case a salt
      *            is generated for you using {@link SecureRandom}.
      * @param prefix
-     *            salt prefix
+     *            The salt prefix {@value #APR1_PREFIX}, {@value #MD5_PREFIX}.
      * @return the hash value
      * @throws IllegalArgumentException
      *             if the salt does not match the allowed pattern
@@ -261,13 +262,13 @@ public static String md5Crypt(final byte[] keyBytes, final String salt, final St
      *            real salt value without prefix or "rounds=". The salt may be null, in which case a salt
      *            is generated for you using {@link SecureRandom}.
      * @param prefix
-     *            salt prefix
+     *            The salt prefix {@value #APR1_PREFIX}, {@value #MD5_PREFIX}.
      * @param random
      *            the instance of {@link Random} to use for generating the salt.
      *            Consider using {@link SecureRandom} for more secure salts.
      * @return the hash value
      * @throws IllegalArgumentException
-     *             if the salt does not match the allowed pattern
+     *             if the salt or prefix does not match the allowed pattern
      * @throws IllegalArgumentException
      *             when a {@link java.security.NoSuchAlgorithmException} is caught.
      * @since 1.12
@@ -280,6 +281,13 @@ public static String md5Crypt(final byte[] keyBytes, final String salt, final St
         if (salt == null) {
             saltString = B64.getRandomSalt(8, random);
         } else {
+            Objects.requireNonNull(prefix, "prefix");
+            if (prefix.length() < 3) {
+                throw new IllegalArgumentException("Invalid prefix value: " + prefix);
+            }
+            if (prefix.charAt(0) != '$' && prefix.charAt(prefix.length() - 1) != '$') {
+                throw new IllegalArgumentException("Invalid prefix value: " + prefix);
+            }
             final Pattern p = Pattern.compile("^" + prefix.replace("$", "\\$") + "([\\.\\/a-zA-Z0-9]{1,8}).*");
             final Matcher m = p.matcher(salt);
             if (!m.find()) {
diff --git a/src/test/java/org/apache/commons/codec/digest/Md5CryptTest.java b/src/test/java/org/apache/commons/codec/digest/Md5CryptTest.java
@@ -35,6 +35,16 @@ public void testCtor() {
         assertNotNull(new Md5Crypt()); // for code-coverage
     }
 
+    @Test
+    public void testInvalidPrefix() {
+        assertThrows(IllegalArgumentException.class, () -> Md5Crypt.md5Crypt(new byte[] { 1, 2, 3, 4, 5 },
+                "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!", "(.*a){10000}"));
+        assertThrows(IllegalArgumentException.class, () -> Md5Crypt.md5Crypt(new byte[] { 1, 2, 3, 4, 5 },
+                "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!", "$(.*a){10000}$"));
+        assertThrows(IllegalArgumentException.class, () -> Md5Crypt.md5Crypt(new byte[] { 1, 2, 3, 4, 5 },
+                "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "$(.*a){10000}$"));
+    }
+
     @Test
     public void testMd5CryptBytes() {
         // An empty Bytearray equals an empty String
