diff --git a/its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java b/its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java index 3e5bbacc95b..b9c115888cd 100644 --- a/its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java +++ b/its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java @@ -195,7 +195,7 @@ public void javaCheckTestSources() throws Exception { SoftAssertions softly = new SoftAssertions(); softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values()); softly.assertThat(newTotal).isEqualTo(knownTotal); - softly.assertThat(rulesCausingFPs).hasSize(11); + softly.assertThat(rulesCausingFPs).hasSize(10); softly.assertThat(rulesNotReporting).hasSize(10); /** diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json index 3f8c8ab4714..e59c21a4602 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json @@ -1,6 +1,6 @@ { "ruleKey": "S1874", "hasTruePositives": true, - "falseNegatives": 246, - "falsePositives": 1 + "falseNegatives": 257, + "falsePositives": 0 } diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2245.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2245.json index 0cd9168e877..a79ef8558b1 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2245.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2245.json @@ -1,6 +1,6 @@ { "ruleKey": "S2245", "hasTruePositives": true, - "falseNegatives": 24, + "falseNegatives": 25, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2440.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2440.json index d43a7de0aec..0d60d9f0390 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2440.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2440.json @@ -1,6 +1,6 @@ { "ruleKey": "S2440", "hasTruePositives": true, - "falseNegatives": 4, + "falseNegatives": 2, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/java-checks-test-sources/default/pom.xml b/java-checks-test-sources/default/pom.xml index 0b48cbb6ea7..04ee2ae32e5 100644 --- a/java-checks-test-sources/default/pom.xml +++ b/java-checks-test-sources/default/pom.xml @@ -502,7 +502,7 @@ org.apache.commons commons-lang3 - 3.12.0 + 3.17.0 jar provided diff --git a/java-checks-test-sources/default/src/main/java/checks/PseudoRandomCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/PseudoRandomCheckSample.java index 86a8946395b..5068a480955 100644 --- a/java-checks-test-sources/default/src/main/java/checks/PseudoRandomCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/PseudoRandomCheckSample.java @@ -74,6 +74,15 @@ void fun() { // ^^^^^^ } + static String randomStringUtilsInstances(int value) { + return switch (value) { + case 0 -> org.apache.commons.lang3.RandomStringUtils.secureStrong().next(42); // Compliant + case 42 -> org.apache.commons.lang3.RandomStringUtils.secure().next(42); // Compliant + default -> org.apache.commons.lang3.RandomStringUtils.insecure().next(42); // Noncompliant + // ^^^^^^^^ + }; + } + int nextInt() { return 42; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java index e877edebb5a..a9387501b7d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java @@ -34,6 +34,8 @@ public class PseudoRandomCheck extends IssuableSubscriptionVisitor { private static final String MESSAGE = "Make sure that using this pseudorandom number generator is safe here."; + private static final String LANG3_RANDOM_STRING_UTILS = "org.apache.commons.lang3.RandomStringUtils"; + private static final MethodMatchers STATIC_RANDOM_METHODS = MethodMatchers.or( MethodMatchers.create().ofTypes("java.lang.Math").names("random").addWithoutParametersMatcher().build(), MethodMatchers.create() @@ -41,14 +43,20 @@ public class PseudoRandomCheck extends IssuableSubscriptionVisitor { "org.apache.commons.lang.math.RandomUtils", "org.apache.commons.lang3.RandomUtils", "org.apache.commons.lang.RandomStringUtils", - "org.apache.commons.lang3.RandomStringUtils") + LANG3_RANDOM_STRING_UTILS) .anyName() .withAnyParameters() .build() ); + private static final MethodMatchers RANDOM_STRING_UTILS_SECURE_INSTANCES = MethodMatchers.create() + .ofSubTypes(LANG3_RANDOM_STRING_UTILS) + .names("secure", "secureStrong") + .withAnyParameters() + .build(); + private static final MethodMatchers RANDOM_STRING_UTILS_RANDOM_WITH_RANDOM_SOURCE = MethodMatchers.create() - .ofSubTypes("org.apache.commons.lang.RandomStringUtils", "org.apache.commons.lang3.RandomStringUtils") + .ofSubTypes("org.apache.commons.lang.RandomStringUtils", LANG3_RANDOM_STRING_UTILS) .names("random") .addParametersMatcher("int", "int", "int", "boolean", "boolean", "char[]", "java.util.Random") .build(); @@ -65,8 +73,7 @@ public List nodesToVisit() { @Override public void visitNode(Tree tree) { - if (tree.is(Tree.Kind.METHOD_INVOCATION)) { - MethodInvocationTree mit = (MethodInvocationTree) tree; + if (tree instanceof MethodInvocationTree mit) { IdentifierTree reportLocation = ExpressionUtils.methodName(mit); if (isStaticCallToInsecureRandomMethod(mit)) { @@ -83,6 +90,7 @@ public void visitNode(Tree tree) { private static boolean isStaticCallToInsecureRandomMethod(MethodInvocationTree mit) { return STATIC_RANDOM_METHODS.matches(mit) && !RANDOM_STRING_UTILS_RANDOM_WITH_RANDOM_SOURCE.matches(mit) + && !RANDOM_STRING_UTILS_SECURE_INSTANCES.matches(mit) && mit.methodSymbol().isStatic(); }