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
The following commit(s) were added to refs/heads/master by this push:
new 6837802c [CODEC-245] Reject malformed Sha2Crypt salt syntax
6837802c is described below
commit 6837802c8f23cb970513035c0fede41a1e30bb23
Author: Gary Gregory <[email protected]>
AuthorDate: Sat Sep 26 20:33:39 2026 +0000
[CODEC-245] Reject malformed Sha2Crypt salt syntax
Refactor tests.
---
.../apache/commons/codec/digest/Sha2CryptTest.java | 81 ++++++++++++++++++++++
.../commons/codec/digest/Sha512CryptTest.java | 9 ---
2 files changed, 81 insertions(+), 9 deletions(-)
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 e94d4e6a..3e14e3bf 100644
--- a/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java
+++ b/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java
@@ -22,18 +22,87 @@ import static
org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
import java.nio.charset.StandardCharsets;
+import java.util.stream.Stream;
+import org.junit.jupiter.api.Named;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
class Sha2CryptTest {
+ private static void assertSaltRejected(final int bits, final String salt) {
+ final byte[] key = "secret".getBytes(StandardCharsets.UTF_8);
+ assertThrowsExactly(IllegalArgumentException.class, () -> {
+ if (bits == 256) {
+ Sha2Crypt.sha256Crypt(key, salt);
+ } else {
+ Sha2Crypt.sha512Crypt(key, salt);
+ }
+ });
+ }
+
+ static Stream<Arguments> invalidSaltCharacters() {
+ return saltsForBothVariants(new String[][] {
+ // @formatter:off
+ { "Unicode after valid prefix", "rounds=1000$abcäöüäöü" },
+ { "space after valid prefix", "abc def" },
+ { "tab after valid prefix", "abc\tdef" },
+ { "punctuation after valid prefix", "abc!" },
+ { "invalid character after 16 valid characters",
"abcdefghijklmnop!" }
+ // @formatter:on
+ });
+ }
+
+ static Stream<Arguments> malformedRounds() {
+ return saltsForBothVariants(new String[][] {
+ // @formatter:off
+ { "incorrect rounds keyword", "notrounds=1000$asdfasdf" },
+ { "empty rounds", "rounds=$abc" },
+ { "non-numeric rounds", "rounds=abc$abc" },
+ { "negative rounds", "rounds=-1$abc" }
+ // @formatter:on
+ });
+ }
+
+ private static Stream<Arguments> saltsForBothVariants(final String[][]
cases) {
+ return Stream.of(256, 512).flatMap(bits -> Stream.of(cases)
+ .map(testCase -> Arguments.of(bits, Named.of(testCase[0],
(bits == 256 ? "$5$" : "$6$") + testCase[1]))));
+ }
+
+ static Stream<Arguments> trailingLineTerminators() {
+ return saltsForBothVariants(new String[][] {
+ // @formatter:off
+ { "LF", "rounds=1000$abc\n" },
+ { "CR", "rounds=1000$abc\r" },
+ { "CRLF", "rounds=1000$abc\r\n" },
+ { "NEL (U+0085)", "rounds=1000$abc\u0085" },
+ { "LINE SEPARATOR (U+2028)", "rounds=1000$abc\u2028" },
+ { "PARAGRAPH SEPARATOR (U+2029)", "rounds=1000$abc\u2029" }
+ // @formatter:on
+ });
+ }
+
+ @SuppressWarnings("deprecation")
@Test
void testCtor() {
assertNotNull(new Sha2Crypt());
}
+ @ParameterizedTest(name = "SHA-{0}: {1}")
+ @MethodSource("invalidSaltCharacters")
+ void testInvalidSaltCharactersRejected(final int bits, final String salt) {
+ assertSaltRejected(bits, salt);
+ }
+
+ @ParameterizedTest(name = "SHA-{0}: invalid salt prefix")
+ @ValueSource(ints = { 256, 512 })
+ void testInvalidSaltPrefixRejected(final int bits) {
+ assertSaltRejected(bits, "xx");
+ }
+
@ParameterizedTest
@ValueSource(ints = { 100_000, 1_000_000 })
void testLargeRounds(final int rounds) {
@@ -41,6 +110,12 @@ class Sha2CryptTest {
Crypt.crypt("anything".getBytes(StandardCharsets.UTF_8), salt);
}
+ @ParameterizedTest(name = "SHA-{0}: {1}")
+ @MethodSource("malformedRounds")
+ void testMalformedRoundsRejected(final int bits, final String salt) {
+ assertSaltRejected(bits, salt);
+ }
+
@Test
void testRoundsAboveCeilingRejected() {
assertThrowsExactly(IllegalArgumentException.class,
@@ -72,4 +147,10 @@ class Sha2CryptTest {
final String actual =
Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8),
"$6$rounds=0000001000$abcdefghijklmnop");
assertEquals(expected, actual);
}
+
+ @ParameterizedTest(name = "SHA-{0}: {1}")
+ @MethodSource("trailingLineTerminators")
+ void testTrailingLineTerminatorsRejected(final int bits, final String
salt) {
+ assertSaltRejected(bits, salt);
+ }
}
diff --git a/src/test/java/org/apache/commons/codec/digest/Sha512CryptTest.java
b/src/test/java/org/apache/commons/codec/digest/Sha512CryptTest.java
index eaf2605c..76142635 100644
--- a/src/test/java/org/apache/commons/codec/digest/Sha512CryptTest.java
+++ b/src/test/java/org/apache/commons/codec/digest/Sha512CryptTest.java
@@ -47,15 +47,6 @@ class Sha512CryptTest {
assertEquals("$5$rounds=9999$abcd$Rh/8ngVh9oyuS6lL3.fsq.9xbvXJsfyKWxSjO2mPIa7",
Sha2Crypt.sha256Crypt("secret".getBytes(StandardCharsets.UTF_8),
"$5$rounds=9999$abcd"));
}
- @Test
- void testSha2CryptWrongSalt() {
- assertThrows(IllegalArgumentException.class, () ->
Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "xx"));
- assertThrows(IllegalArgumentException.class,
- () ->
Sha2Crypt.sha256Crypt("secret".getBytes(StandardCharsets.UTF_8),
"$5$notrounds=1000$asdfasdf"));
- assertThrows(IllegalArgumentException.class,
- () ->
Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8),
"$6$rounds=1000$abcäöüäöü"));
- }
-
@Test
void testSha512CryptBytes() {
// An empty byte array equals an empty String
