This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-codec.git
commit 1d62be3ed1139336da8c71538b9911e218f53c29 Author: Gary Gregory <[email protected]> AuthorDate: Sat Sep 19 05:42:33 2026 -0700 Bound Sha2Crypt rounds from caller-supplied salts Limit input-derived SHA-256/SHA-512 crypt rounds to 1,000,000 by default to prevent attacker-controlled CPU exhaustion. Support an explicit system-property override for compatibility, safely reject oversized values, preserve leading-zero parsing, and document the untrusted work factor in Crypt verification. --- src/changes/changes.xml | 1 + .../org/apache/commons/codec/digest/Crypt.java | 6 ++++ .../org/apache/commons/codec/digest/Sha2Crypt.java | 33 ++++++++++++++++-- .../apache/commons/codec/digest/Sha2CryptTest.java | 40 +++++++++++++++++++--- 4 files changed, 73 insertions(+), 7 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 320900e9..da5537ae 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -65,6 +65,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Validate BinaryCodec input while preserving leading-bit truncation.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Reject oversized Beider-Morse input before language guessing.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">QuotedPrintableCodec decoding now preserves hard CRLF line breaks and, leniently, unpaired CR and LF bytes instead of discarding them. Soft line breaks require the full =CRLF sequence; previously accepted =CR without LF now throws DecoderException. This affects both constructor modes and QCodec, which shares the decoder.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Bound Sha2Crypt rounds from caller-supplied salts.</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use PhoneticEngine.Builder and deprecate old constructors.</action> <action type="add" dev="ggregory" due-to="Gary Gregory">Add BeiderMorseEncoder.Builder and deprecate old constructor.</action> diff --git a/src/main/java/org/apache/commons/codec/digest/Crypt.java b/src/main/java/org/apache/commons/codec/digest/Crypt.java index 30565548..b7514248 100644 --- a/src/main/java/org/apache/commons/codec/digest/Crypt.java +++ b/src/main/java/org/apache/commons/codec/digest/Crypt.java @@ -122,6 +122,12 @@ public class Crypt { * storedPwd.equals(crypt(enteredPwd, storedPwd)) * </pre> * <p> + * For SHA-256 and SHA-512 crypt strings, the stored value can include a {@code rounds=} work factor. Treat a complete crypt string as untrusted input when + * verifying passwords. {@link Sha2Crypt} limits caller-supplied work factors to 1,000,000 rounds by default. Applications that deliberately need a higher + * limit can set the {@code org.apache.commons.codec.digest.Sha2Crypt.roundsMax} system property, up to the crypt specification maximum, and should apply + * their own authentication timeouts and resource controls. + * </p> + * <p> * The resulting string starts with the marker string ({@code $n$}), where n is the same as the input salt. The salt is then appended, followed by a * {@code "$"} sign. This is followed by the actual hash value. For DES the string only contains the salt and actual hash. The total length is dependent on * the algorithm used: diff --git a/src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java b/src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java index 9c9212d4..ab70935e 100644 --- a/src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java +++ b/src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java @@ -49,12 +49,18 @@ public class Sha2Crypt { /** Maximum number of rounds. */ private static final int ROUNDS_MAX = 999_999_999; + /** Default maximum number of rounds accepted from a caller-supplied salt string. */ + private static final int ROUNDS_MAX_DEFAULT = 1_000_000; + /** Minimum number of rounds. */ private static final int ROUNDS_MIN = 1000; /** Prefix for optional rounds specification. */ private static final String ROUNDS_PREFIX = "rounds="; + /** System property used to override the default maximum number of rounds. */ + static final String ROUNDS_MAX_PROPERTY = "org.apache.commons.codec.digest.Sha2Crypt.roundsMax"; + /** The number of bytes the final hash value will have (SHA-256 variant). */ private static final int SHA256_BLOCKSIZE = 32; @@ -167,8 +173,17 @@ public class Sha2Crypt { throw new IllegalArgumentException("Invalid salt value: " + salt); } if (m.group(3) != null) { - rounds = Integer.parseInt(m.group(3)); - rounds = Math.max(ROUNDS_MIN, Math.min(ROUNDS_MAX, rounds)); + final int roundsMax = Math.max(ROUNDS_MIN, + Math.min(ROUNDS_MAX, Integer.getInteger(ROUNDS_MAX_PROPERTY, ROUNDS_MAX_DEFAULT))); + final String roundsString = m.group(3); + final int firstNonZero = firstNonZeroIndex(roundsString); + final String normalizedRounds = roundsString.substring(firstNonZero); + final String roundsMaxString = Integer.toString(roundsMax); + if (normalizedRounds.length() > roundsMaxString.length() + || normalizedRounds.length() == roundsMaxString.length() && normalizedRounds.compareTo(roundsMaxString) > 0) { + throw new IllegalArgumentException("Rounds value in salt exceeds the maximum of " + roundsMax + ": " + salt); + } + rounds = Math.max(ROUNDS_MIN, Integer.parseInt(normalizedRounds)); roundsCustom = true; } final String saltString = m.group(4); @@ -526,6 +541,20 @@ public class Sha2Crypt { return buffer.toString(); } + /** + * Finds the first non-zero digit, retaining one zero for an all-zero value. + * + * @param value a non-empty decimal string + * @return the index of the first significant digit + */ + private static int firstNonZeroIndex(final String value) { + int index = 0; + while (index < value.length() - 1 && value.charAt(index) == '0') { + index++; + } + return index; + } + /** * Generates a libc crypt() compatible "$6$" hash value with random salt. * diff --git a/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java b/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java index 097ef9a0..13e7507b 100644 --- a/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java +++ b/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java @@ -17,7 +17,9 @@ package org.apache.commons.codec.digest; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; import java.nio.charset.StandardCharsets; @@ -33,13 +35,41 @@ class Sha2CryptTest { } @ParameterizedTest - @ValueSource(ints = { 100_000, 1_000_000, 5_000_000 /*, 50_000_000*/ }) + @ValueSource(ints = { 100_000, 1_000_000 }) void testLargeRounds(final int rounds) { final String salt = "$6$rounds=" + rounds + "$abcdefghijklmnop"; - final long t = System.nanoTime(); Crypt.crypt("anything".getBytes(StandardCharsets.UTF_8), salt); - Crypt.crypt("anything".getBytes(StandardCharsets.UTF_8), "$6$rounds=5000000$abcdefghijklmnop"); - // Full effect (WARNING: ~2 min): - // Crypt.crypt("anything".getBytes(), "$6$rounds=999999999$abcdefghijklmnop"); + } + + @Test + void testRoundsAboveCeilingRejected() { + assertThrowsExactly(IllegalArgumentException.class, + () -> Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=1000001$abcdefghijklmnop")); + assertThrowsExactly(IllegalArgumentException.class, + () -> Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=999999999$abcdefghijklmnop")); + assertThrowsExactly(IllegalArgumentException.class, + () -> Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=99999999999$abcdefghijklmnop")); + } + + @Test + void testRoundsLeadingZeroes() { + final String expected = Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=1000$abcdefghijklmnop"); + final String actual = Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=0000001000$abcdefghijklmnop"); + assertEquals(expected, actual); + } + + @Test + void testRoundsCeilingOverride() { + final String previous = System.getProperty(Sha2Crypt.ROUNDS_MAX_PROPERTY); + System.setProperty(Sha2Crypt.ROUNDS_MAX_PROPERTY, "2000000"); + try { + assertNotNull(Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=2000000$abcdefghijklmnop")); + } finally { + if (previous == null) { + System.clearProperty(Sha2Crypt.ROUNDS_MAX_PROPERTY); + } else { + System.setProperty(Sha2Crypt.ROUNDS_MAX_PROPERTY, previous); + } + } } }
