SONARJAVA-4858 S5344: fix support of NoOpPasswordEncoder and SCryptPa…

…sswordEncoder (#4737)

its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json

@@ -1,6 +1,6 @@
 {
   "ruleKey": "S1874",
   "hasTruePositives": true,
-  "falseNegatives": 108,
+  "falseNegatives": 114,
   "falsePositives": 0
 }

its/autoscan/src/test/resources/autoscan/diffs/diff_S4790.json

@@ -1,6 +1,6 @@
 {
   "ruleKey": "S4790",
   "hasTruePositives": true,
-  "falseNegatives": 37,
+  "falseNegatives": 43,
   "falsePositives": 0
 }

its/autoscan/src/test/resources/autoscan/diffs/diff_S5344.json

@@ -1,6 +1,6 @@
 {
   "ruleKey": "S5344",
   "hasTruePositives": false,
-  "falseNegatives": 15,
+  "falseNegatives": 23,
   "falsePositives": 0
 }

its/autoscan/src/test/resources/autoscan/diffs/diff_S6437.json

@@ -1,6 +1,6 @@
 {
   "ruleKey": "S6437",
   "hasTruePositives": true,
-  "falseNegatives": 57,
+  "falseNegatives": 58,
   "falsePositives": 0
 }

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<String, PasswordEncoder> 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");
+  }
+}

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<Tree.Kind> 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);

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();
   }
-
 }