From 7f5067a97ea5356f2f2c27c3f99305b65518d374 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Tue, 25 Aug 2026 14:57:28 +0200 Subject: [PATCH] SONARJAVA-6853 S3329: fix FP when cipher.init uses a non-final variable initialized to DECRYPT_MODE The DECRYPT_MODE suppression in SecureInitializationFinder relied on asConstant() to resolve the operation mode, which only works for compile-time constants. A non-final variable initialized to Cipher.DECRYPT_MODE was not resolved, causing the suppression to silently fail and a FP to be raised. Added resolveIntConstant() which falls back to inspecting the variable's declaration initializer when asConstant() returns empty. Co-Authored-By: Claude Sonnet 4.6 --- ...ningCheckShouldDetectCustomIVFactories.java | 5 +++++ .../security/CipherBlockChainingCheck.java | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/security/CipherBlockChainingCheckShouldDetectCustomIVFactories.java b/java-checks-test-sources/default/src/main/java/checks/security/CipherBlockChainingCheckShouldDetectCustomIVFactories.java index 236486cb5d1..0168654842c 100644 --- a/java-checks-test-sources/default/src/main/java/checks/security/CipherBlockChainingCheckShouldDetectCustomIVFactories.java +++ b/java-checks-test-sources/default/src/main/java/checks/security/CipherBlockChainingCheckShouldDetectCustomIVFactories.java @@ -189,6 +189,11 @@ public void calls_method_to_generate_iv() final byte[] iv = generateRandomData(12); // An FP used to be raised here because iv is securely generated in a separate method, and we were not able to trace that. cipher.init(ENCRYPT_MODE, secretKey, new IvParameterSpec(iv)); // Compliant + + // new byte[16] is not dynamically generated, so the DECRYPT_MODE suppression path must fire. + // No FP should be raised: opMode1's initializer resolves to DECRYPT_MODE even though opMode1 is non-final. + int opMode1 = Cipher.DECRYPT_MODE; + cipher.init(opMode1, secretKey, new IvParameterSpec(new byte[16])); } private byte[] generateRandomData(final int length) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/CipherBlockChainingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/CipherBlockChainingCheck.java index da92e5d286c..e3079cda16f 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/CipherBlockChainingCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/CipherBlockChainingCheck.java @@ -20,6 +20,7 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -191,7 +192,7 @@ public void visitMethodInvocation(MethodInvocationTree methodInvocation) { } // make sure it is not used for decryption - in such case you need to reuse one if (CIPHER_INIT.matches(methodInvocation) && methodInvocation.arguments().size() > 2) { - int opMode = methodInvocation.arguments().get(0).asConstant(Integer.class).orElse(-1); + int opMode = resolveIntConstant(methodInvocation.arguments().get(0)).orElse(-1); if (CIPHER_INIT_DECRYPT_MODE == opMode && isPartOfArguments(methodInvocation)) { hasBeenSecurelyInitialized = true; } @@ -241,6 +242,21 @@ private static boolean isPartOfArguments(MethodInvocationTree methodInvocation, .anyMatch(ivParameterSymbol::equals); } + private static Optional resolveIntConstant(ExpressionTree expression) { + Optional constant = expression.asConstant(Integer.class); + if (constant.isPresent()) { + return constant; + } + if (expression instanceof IdentifierTree identifierTree + && identifierTree.symbol() instanceof Symbol.VariableSymbol variableSymbol) { + var declaration = variableSymbol.declaration(); + if (declaration != null && declaration.initializer() != null) { + return declaration.initializer().asConstant(Integer.class); + } + } + return Optional.empty(); + } + private static Symbol symbol(ExpressionTree expression) { if (expression.is(Tree.Kind.IDENTIFIER)) { return ((IdentifierTree) expression).symbol();