SONARJAVA-5818: Fix FPs caused by missing state reset in CipherBlockChainingCheck

Author: anton-haubner-sonarsource

java-checks-test-sources/default/src/main/java/checks/security/CipherBlockChainingCheckShouldResetState.java

@@ -0,0 +1,37 @@
+package checks.security;
+
+import java.security.InvalidAlgorithmParameterException;
+import java.security.InvalidKeyException;
+import java.security.SecureRandom;
+import javax.crypto.Cipher;
+import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+
+import static javax.crypto.Cipher.ENCRYPT_MODE;
+
+// This file contains some trivial cases to check that CipherBlockChainingCheck resets its class/file-level state properly.
+// For this purpose, the tests of CipherBlockChainingCheck analyze this file immediately after and before the main test sources.
+//
+// See the other test sources for proper tests of CipherBlockChainingCheck.
+public class CipherBlockChainingCheckShouldResetState {
+  static Cipher cipher;
+  static SecretKeySpec secretKey;
+
+  void tn_control() throws InvalidAlgorithmParameterException, InvalidKeyException {
+    final byte[] iv = secureIv();
+    cipher.init(ENCRYPT_MODE, secretKey, new IvParameterSpec(iv)); // Compliant
+  }
+
+  void tp_control() throws InvalidAlgorithmParameterException, InvalidKeyException {
+    final byte[] iv = insecureIv();
+    cipher.init(ENCRYPT_MODE, secretKey, new IvParameterSpec(iv)); // Noncompliant
+  }
+
+  private byte[] secureIv() {
+    return new SecureRandom().generateSeed(12);
+  }
+
+  private byte[] insecureIv() {
+    return new byte[12];
+  }
+}

java-checks/src/main/java/org/sonar/java/checks/security/CipherBlockChainingCheck.java

@@ -100,6 +100,7 @@ public void visitNode(Tree tree) {
   public void leaveNode(Tree tree) {
     if (tree == outermostClass) {
       ivFactoryFinder.clear();
+      outermostClass = null;
     }
     super.leaveNode(tree);
   }

java-checks/src/test/java/org/sonar/java/checks/security/CipherBlockChainingCheckTest.java

@@ -26,6 +26,7 @@ class CipherBlockChainingCheckTest {
   private static final String BASE_PATH = "checks/security";
   private static final String DEFAULT_SOURCE_PATH = BASE_PATH + "/CipherBlockChainingCheck.java";
   private static final String CUSTOM_IV_FACTORY_DETECTION_SOURCE_PATH = BASE_PATH + "/CipherBlockChainingCheckShouldDetectCustomIVFactories.java";
+  private static final String STATE_RESET_TEST_FILE_SOURCE_PATH = BASE_PATH + "/CipherBlockChainingCheckShouldResetState.java";
 
   @Test
   void test() {
@@ -58,4 +59,28 @@ void should_detect_custom_iv_factories_non_compiling() {
       .withCheck(new CipherBlockChainingCheck())
       .verifyIssues();
   }
+
+  @Test
+  void should_reset_state_between_class_and_file_analyses() {
+    // CipherBlockChainingCheck needs to properly reset its state in between analyzing classes/files.
+    // Otherwise, it might not refresh its knowledge about secure factory methods that exist in a given class.
+    // This test validates the reset, by analyzing another file before and after the main test source file.
+
+    // First, lets verify that the additional testing file passes the verifier on its own
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath(STATE_RESET_TEST_FILE_SOURCE_PATH))
+      .withCheck(new CipherBlockChainingCheck())
+      .verifyIssues();
+
+    // Then lets interleave its analysis with the main file from above
+    CheckVerifier.newVerifier()
+      .onFiles(mainCodeSourcesPath(STATE_RESET_TEST_FILE_SOURCE_PATH), mainCodeSourcesPath(CUSTOM_IV_FACTORY_DETECTION_SOURCE_PATH))
+      .withCheck(new CipherBlockChainingCheck())
+      .verifyIssues();
+
+    CheckVerifier.newVerifier()
+      .onFiles(mainCodeSourcesPath(CUSTOM_IV_FACTORY_DETECTION_SOURCE_PATH), mainCodeSourcesPath(STATE_RESET_TEST_FILE_SOURCE_PATH))
+      .withCheck(new CipherBlockChainingCheck())
+      .verifyIssues();
+  }
 }