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 0c9bd7ec4a8..cf22ff08e48 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": 108, + "falseNegatives": 114, "falsePositives": 0 } diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S4790.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S4790.json index 1daa224ddb4..4a506a26cce 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S4790.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S4790.json @@ -1,6 +1,6 @@ { "ruleKey": "S4790", "hasTruePositives": true, - "falseNegatives": 37, + "falseNegatives": 43, "falsePositives": 0 } \ No newline at end of file diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5344.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5344.json index 1dc03156fde..e42c7d31060 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5344.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5344.json @@ -1,6 +1,6 @@ { "ruleKey": "S5344", "hasTruePositives": false, - "falseNegatives": 15, + "falseNegatives": 23, "falsePositives": 0 } \ No newline at end of file diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6437.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6437.json index ba4a1761fd1..f7dd855ab4c 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6437.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6437.json @@ -1,6 +1,6 @@ { "ruleKey": "S6437", "hasTruePositives": true, - "falseNegatives": 57, + "falseNegatives": 58, "falsePositives": 0 } \ No newline at end of file diff --git a/java-checks-test-sources/default/src/main/java/checks/security/PasswordEncoder.java b/java-checks-test-sources/default/src/main/java/checks/security/PasswordEncoder.java index 81f3d5c0930..8ba750bf484 100644 --- a/java-checks-test-sources/default/src/main/java/checks/security/PasswordEncoder.java +++ b/java-checks-test-sources/default/src/main/java/checks/security/PasswordEncoder.java @@ -1,14 +1,24 @@ package checks.security; +import java.util.HashMap; +import java.util.Map; import javax.sql.DataSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; +import org.springframework.security.crypto.password.DelegatingPasswordEncoder; +import org.springframework.security.crypto.password.LdapShaPasswordEncoder; +import org.springframework.security.crypto.password.Md4PasswordEncoder; import org.springframework.security.crypto.password.MessageDigestPasswordEncoder; +import org.springframework.security.crypto.password.NoOpPasswordEncoder; +import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.crypto.password.Pbkdf2PasswordEncoder; import org.springframework.security.crypto.password.StandardPasswordEncoder; +import org.springframework.security.crypto.scrypt.SCryptPasswordEncoder; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; @EnableWebSecurity class SecurityConfig { @@ -66,3 +76,23 @@ public void ldapAuthentication(AuthenticationManagerBuilder auth) throws Excepti auth.ldapAuthentication().passwordEncoder(new MessageDigestPasswordEncoder("MD5")); // Noncompliant } } + +@RestController +class RestControllerPasswordEncoderSample { + + @GetMapping(value = "/passwordEncoder2") + public String passwordEncoder() { + Map encoders = new HashMap<>(); + encoders.put("noop", NoOpPasswordEncoder.getInstance()); // Noncompliant + encoders.put("md4", new Md4PasswordEncoder()); // Noncompliant + encoders.put("md5", new MessageDigestPasswordEncoder("md5")); // Noncompliant + encoders.put("SHA-1", new MessageDigestPasswordEncoder("SHA-1")); // Noncompliant + encoders.put("ldap", new LdapShaPasswordEncoder()); // Noncompliant + encoders.put("sha-256", new StandardPasswordEncoder()); // Noncompliant + encoders.put("scrypt", new SCryptPasswordEncoder(10,10,10,10,10)); // Noncompliant + + PasswordEncoder passwordEncoder = new DelegatingPasswordEncoder("noop", encoders); + + return passwordEncoder.encode("Password"); + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/PasswordEncoderCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/PasswordEncoderCheck.java index 6b7d8c3afa1..72a72ab4afb 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/PasswordEncoderCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/PasswordEncoderCheck.java @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.List; import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.tree.BaseTreeVisitor; @@ -50,29 +51,36 @@ public class PasswordEncoderCheck extends IssuableSubscriptionVisitor { .withAnyParameters() .build(); - private static final MethodMatchers UNSAFE_PASSWORD_ENCODERS = MethodMatchers.create() + private static final MethodMatchers UNSAFE_PASSWORD_ENCODER_CONSTRUCTORS = MethodMatchers.create() .ofTypes( "org.springframework.security.authentication.encoding.ShaPasswordEncoder", "org.springframework.security.authentication.encoding.Md5PasswordEncoder", "org.springframework.security.crypto.password.LdapShaPasswordEncoder", "org.springframework.security.crypto.password.Md4PasswordEncoder", "org.springframework.security.crypto.password.MessageDigestPasswordEncoder", - "org.springframework.security.crypto.password.NoOpPasswordEncoder", "org.springframework.security.crypto.password.StandardPasswordEncoder", - "org.springframework.security.crypto.password.SCryptPasswordEncoder") + "org.springframework.security.crypto.scrypt.SCryptPasswordEncoder") .constructor() .withAnyParameters() .build(); + private static final MethodMatchers UNSAFE_PASSWORD_ENCODER_METHODS = MethodMatchers.create() + .ofTypes("org.springframework.security.crypto.password.NoOpPasswordEncoder") + .names("getInstance") + .addWithoutParametersMatcher() + .build(); + @Override public List nodesToVisit() { - return Arrays.asList(Tree.Kind.METHOD, Tree.Kind.NEW_CLASS); + return Arrays.asList(Tree.Kind.METHOD, Tree.Kind.NEW_CLASS, Tree.Kind.METHOD_INVOCATION); } @Override public void visitNode(Tree tree) { - if (tree.is(Tree.Kind.NEW_CLASS) && UNSAFE_PASSWORD_ENCODERS.matches(((NewClassTree) tree))) { - reportIssue(((NewClassTree) tree).identifier(), "Use secure \"PasswordEncoder\" implementation."); + if (tree instanceof NewClassTree nct && UNSAFE_PASSWORD_ENCODER_CONSTRUCTORS.matches(nct)) { + reportIssue(nct.identifier(), "Use secure \"PasswordEncoder\" implementation."); + } else if (tree instanceof MethodInvocationTree mit && UNSAFE_PASSWORD_ENCODER_METHODS.matches(mit)) { + reportIssue(ExpressionUtils.methodName(mit), "Use secure \"PasswordEncoder\" implementation."); } else if (tree.is(Tree.Kind.METHOD)) { MethodInvocationVisitor visitor = new MethodInvocationVisitor(); tree.accept(visitor); diff --git a/java-checks/src/test/java/org/sonar/java/checks/security/PasswordEncoderCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/security/PasswordEncoderCheckTest.java index e237ad75170..4f5bce43152 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/security/PasswordEncoderCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/security/PasswordEncoderCheckTest.java @@ -32,11 +32,14 @@ void test() { .onFile(mainCodeSourcesPath("checks/security/PasswordEncoder.java")) .withCheck(new PasswordEncoderCheck()) .verifyIssues(); + } + + @Test + void test_no_semantics() { CheckVerifier.newVerifier() .onFile(mainCodeSourcesPath("checks/security/PasswordEncoder.java")) .withCheck(new PasswordEncoderCheck()) .withoutSemantic() .verifyNoIssues(); } - }