From b8cc914adce04cd622ab792dfb2298ce98e560ab Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Fri, 21 Aug 2026 10:03:15 +0200 Subject: [PATCH 01/18] SONARJAVA-6783: Implement rule S9345 Classes with throwing constructors should be protected against Finalizer attacks Detect non-final, non-abstract classes whose non-private constructors can throw exceptions (via throws clause or throw statements in the body), making them vulnerable to Finalizer attacks through malicious subclasses. Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 189 ++++++++++++++++++ .../java/checks/FinalizerAttackCheck.java | 92 +++++++++ .../java/checks/FinalizerAttackCheckTest.java | 34 ++++ .../org/sonar/l10n/java/rules/java/S9345.html | 68 +++++++ .../org/sonar/l10n/java/rules/java/S9345.json | 32 +++ 5 files changed, 415 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java new file mode 100644 index 00000000000..02377215ec0 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -0,0 +1,189 @@ +package checks; + +class FinalizerAttackCheckSample { + + // --- Noncompliant: non-final class with throwing constructor --- + + class SecurityService { // Noncompliant {{Make this class "final" or make the throwing constructors "private".}} + private final String token; + + public SecurityService(String token) throws IllegalArgumentException { + if (token == null) { + throw new IllegalArgumentException("Invalid token"); + } + this.token = token; + } + } + + class AuthProvider { // Noncompliant + public AuthProvider(String credentials) throws Exception { + if (credentials.isEmpty()) { + throw new Exception("Bad credentials"); + } + } + } + + class ResourceLoader { // Noncompliant + ResourceLoader(String path) { + if (path == null) { + throw new NullPointerException(); + } + } + } + + class MultiConstructorService { // Noncompliant + MultiConstructorService(int id) throws Exception { + if (id < 0) { + throw new Exception("Negative id"); + } + } + + MultiConstructorService(String name) { + } + } + + class ProtectedConstructorService { // Noncompliant + protected ProtectedConstructorService(String data) throws Exception { + if (data == null) { + throw new Exception("Null data"); + } + } + } + + class ThrowsClauseOnly { // Noncompliant + public ThrowsClauseOnly() throws Exception { + } + } + + // --- Compliant: final class --- + + final class SecureService { + public SecureService(String token) throws IllegalArgumentException { + if (token == null) { + throw new IllegalArgumentException("Invalid token"); + } + } + } + + // --- Compliant: all constructors private (factory pattern) --- + + class FactoryService { + private FactoryService(String data) { + } + + public static FactoryService create(String data) throws Exception { + if (data == null) { + throw new Exception("Null"); + } + return new FactoryService(data); + } + } + + // --- Compliant: no throwing constructor --- + + class SafeService { + public SafeService(String data) { + // no throw + } + } + + class NoConstructor { + void doSomething() { + } + } + + // --- Compliant: abstract class --- + + abstract class AbstractService { + public AbstractService(String data) throws Exception { + if (data == null) { + throw new Exception("Null"); + } + } + } + + // --- Compliant: private throwing constructor, public non-throwing constructor --- + + class MixedConstructors { + private MixedConstructors(String data) throws Exception { + if (data == null) { + throw new Exception("Null"); + } + } + + public MixedConstructors(int id) { + } + } + + // --- Compliant: enum (implicitly final) --- + + enum Status { + ACTIVE, INACTIVE; + + Status() { + } + } + + // --- Compliant: record (implicitly final) --- + + record Credential(String value) { + Credential { + if (value == null) { + throw new IllegalArgumentException("Null value"); + } + } + } + + // --- Compliant: inner interface (no constructors) --- + + interface Service { + void execute(); + } + + // --- Noncompliant: throw in constructor body without throws clause --- + + class ConfigLoader { // Noncompliant + public ConfigLoader(String config) { + if (config == null) { + throw new IllegalStateException("Missing config"); + } + } + } + + // --- Compliant: throw in a method, not in constructor --- + + class Processor { + public Processor() { + } + + public void process() { + throw new UnsupportedOperationException(); + } + } + + // --- Noncompliant: nested throw in try block within constructor --- + + class DatabaseConnection { // Noncompliant + public DatabaseConnection(String url) { + try { + if (url == null) { + throw new RuntimeException("Null URL"); + } + } catch (Exception e) { + throw new RuntimeException("Connection failed", e); + } + } + } + + // --- Compliant: all throwing constructors are private --- + + class PrivateOnlyThrowers { + private PrivateOnlyThrowers(String s) throws Exception { + throw new Exception(); + } + + private PrivateOnlyThrowers(int i) { + throw new IllegalArgumentException(); + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java new file mode 100644 index 00000000000..3300f4929b9 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -0,0 +1,92 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.Collections; +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.java.model.ModifiersUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.BaseTreeVisitor; +import org.sonar.plugins.java.api.tree.BlockTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.LambdaExpressionTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Modifier; +import org.sonar.plugins.java.api.tree.ThrowStatementTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.Tree.Kind; + +@Rule(key = "S9345") +public class FinalizerAttackCheck extends IssuableSubscriptionVisitor { + + @Override + public List nodesToVisit() { + return Collections.singletonList(Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + ClassTree classTree = (ClassTree) tree; + if (ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.FINAL) || + ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.ABSTRACT)) { + return; + } + for (Tree member : classTree.members()) { + if (member.is(Kind.CONSTRUCTOR) && isVulnerableConstructor((MethodTree) member)) { + reportIssue(classTree.simpleName(), "Make this class \"final\" or make the throwing constructors \"private\"."); + return; + } + } + } + + private static boolean isVulnerableConstructor(MethodTree constructor) { + if (ModifiersUtils.hasModifier(constructor.modifiers(), Modifier.PRIVATE)) { + return false; + } + return !constructor.throwsClauses().isEmpty() || containsThrowStatement(constructor); + } + + private static boolean containsThrowStatement(MethodTree constructor) { + BlockTree block = constructor.block(); + if (block == null) { + return false; + } + ThrowStatementVisitor visitor = new ThrowStatementVisitor(); + block.accept(visitor); + return visitor.hasThrow; + } + + private static class ThrowStatementVisitor extends BaseTreeVisitor { + boolean hasThrow; + + @Override + public void visitThrowStatement(ThrowStatementTree tree) { + hasThrow = true; + } + + @Override + public void visitClass(ClassTree tree) { + // skip nested classes + } + + @Override + public void visitLambdaExpression(LambdaExpressionTree tree) { + // skip lambdas + } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java new file mode 100644 index 00000000000..fb9fbd09c12 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java @@ -0,0 +1,34 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class FinalizerAttackCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/FinalizerAttackCheckSample.java")) + .withCheck(new FinalizerAttackCheck()) + .verifyIssues(); + } + +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html new file mode 100644 index 00000000000..ff87bbc6281 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html @@ -0,0 +1,68 @@ +

Why is this an issue?

+

When a constructor throws an exception, you might expect the object construction to fail completely and no reference to the object to exist. However, +finalization or cleanup mechanisms can be exploited to obtain a reference to a partially-constructed object.

+

Here's how a Finalizer attack works:

+
    +
  1. An attacker creates a malicious derived class that overrides the cleanup/finalization method
    2. +
  2. The attacker attempts to instantiate this derived class
    3. +
  3. If the parent constructor throws an exception during initialization, the object is not fully constructed
    4. +
  4. Despite the exception, the garbage collector will eventually call the finalization method on the partially-constructed object
    5. +
  5. The malicious cleanup method can store a reference to the object being finalized, effectively "resurrecting" the broken object
    6. +
  6. The attacker now has access to an object that bypassed security checks or validation logic in the constructor
    7. +
+

This vulnerability is particularly dangerous for security-sensitive classes where the constructor performs authentication or authorization checks, +input validation, resource allocation with security constraints, or initialization of security-critical fields.

+

How to fix it

+

The simplest solution is to declare the class as final. This prevents attackers from creating malicious subclasses that override the +finalize() method.

+

However, some frameworks such as Spring or JPA/Hibernate require non-final classes. In such cases, use a factory method with a private +constructor to ensure the object is fully validated before any reference is exposed. Since the constructor is private, no malicious subclass +can be created, achieving the same protection as final.

+

Noncompliant code example

+
+public class SecuritySensitiveClass {
+    private final String credentials;
+
+    public SecuritySensitiveClass(String credentials) throws AuthenticationException {
+        if (!isValid(credentials)) {
+            throw new AuthenticationException("Invalid credentials"); // Noncompliant
+        }
+        this.credentials = credentials;
+    }
+
+    private boolean isValid(String credentials) {
+        return credentials != null && credentials.length() > 10;
+    }
+}
+
+

Compliant solution

+
+public final class SecuritySensitiveClass { // Compliant: class is final
+    private final String credentials;
+
+    public SecuritySensitiveClass(String credentials) throws AuthenticationException {
+        if (!isValid(credentials)) {
+            throw new AuthenticationException("Invalid credentials");
+        }
+        this.credentials = credentials;
+    }
+
+    private boolean isValid(String credentials) {
+        return credentials != null && credentials.length() > 10;
+    }
+}
+
+

Resources

+

Documentation

+ +

Standards

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json new file mode 100644 index 00000000000..87c32a9e4cf --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json @@ -0,0 +1,32 @@ +{ + "title": "Classes with throwing constructors should be protected against Finalizer attacks", + "type": "VULNERABILITY", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "cert", + "cwe", + "serialization" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-9345", + "sqKey": "S9345", + "scope": "Main", + "defaultQualityProfiles": [ + "Sonar way" + ], + "quickfix": "unknown", + "code": { + "impacts": { + "SECURITY": "HIGH" + }, + "attribute": "COMPLETE" + }, + "securityStandards": { + "CWE": [586], + "CERT": ["OBJ11-J."] + } +} From 50133ac7b68145fec4a53869744bff0cff534583 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 21 Aug 2026 08:19:16 +0000 Subject: [PATCH 02/18] Update ruling results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with GitHub Actions --- .../commons-beanutils/java-S9345.json | 32 ++++++ .../resources/eclipse-jetty/java-S9345.json | 107 ++++++++++++++++++ .../src/test/resources/guava/java-S9345.json | 8 ++ .../resources/sonar-server/java-S9345.json | 23 ++++ 4 files changed, 170 insertions(+) create mode 100644 its/ruling/src/test/resources/commons-beanutils/java-S9345.json create mode 100644 its/ruling/src/test/resources/eclipse-jetty/java-S9345.json create mode 100644 its/ruling/src/test/resources/guava/java-S9345.json create mode 100644 its/ruling/src/test/resources/sonar-server/java-S9345.json diff --git a/its/ruling/src/test/resources/commons-beanutils/java-S9345.json b/its/ruling/src/test/resources/commons-beanutils/java-S9345.json new file mode 100644 index 00000000000..efeec87b4a1 --- /dev/null +++ b/its/ruling/src/test/resources/commons-beanutils/java-S9345.json @@ -0,0 +1,32 @@ +{ +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanPropertyValueChangeClosure.java": [ +79 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanPropertyValueEqualsPredicate.java": [ +110 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanToPropertyValueTransformer.java": [ +71 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/FluentPropertyBeanIntrospector.java": [ +78 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java": [ +44 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/MethodUtils.java": [ +1304 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/ResultSetDynaClass.java": [ +82 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/RowSetDynaClass.java": [ +66 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/SuppressPropertiesBeanIntrospector.java": [ +38 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/converters/ArrayConverter.java": [ +129 +] +} diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json new file mode 100644 index 00000000000..2ef088f3283 --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json @@ -0,0 +1,107 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java": [ +28 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ +32 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java": [ +30 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java": [ +41 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java": [ +39 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java": [ +273 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java": [ +41 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java": [ +29 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java": [ +33 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java": [ +83 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java": [ +38 +], +"org.eclipse.jetty:jetty-project:jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONPojoConvertorFactory.java": [ +29 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/BlockingArrayQueue.java": [ +49 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ClassLoadingObjectInputStream.java": [ +32 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/CountingCallback.java": [ +41 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java": [ +26 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java": [ +39 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/InetAddressPattern.java": [ +110, +191, +236 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java": [ +29 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartWriter.java": [ +28 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java": [ +38, +154 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/PathWatcher.java": [ +70 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java": [ +37 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/RolloverFileOutputStream.java": [ +51 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/Uptime.java": [ +36 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/component/FileDestroyable.java": [ +32 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java": [ +53 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java": [ +43 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/security/CertificateValidator.java": [ +55 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java": [ +40 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/X509.java": [ +37 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java": [ +48 +], +"org.eclipse.jetty:jetty-project:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlAppendable.java": [ +30 +], +"org.eclipse.jetty:jetty-project:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java": [ +87 +] +} diff --git a/its/ruling/src/test/resources/guava/java-S9345.json b/its/ruling/src/test/resources/guava/java-S9345.json new file mode 100644 index 00000000000..68013f90b30 --- /dev/null +++ b/its/ruling/src/test/resources/guava/java-S9345.json @@ -0,0 +1,8 @@ +{ +"com.google.guava:guava:src/com/google/common/base/FinalizableReferenceQueue.java": [ +94 +], +"com.google.guava:guava:src/com/google/common/io/MultiReader.java": [ +33 +] +} diff --git a/its/ruling/src/test/resources/sonar-server/java-S9345.json b/its/ruling/src/test/resources/sonar-server/java-S9345.json new file mode 100644 index 00000000000..bcad8a0b67f --- /dev/null +++ b/its/ruling/src/test/resources/sonar-server/java-S9345.json @@ -0,0 +1,23 @@ +{ +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/source/ReportIterator.java": [ +33 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java": [ +53 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/platform/web/MasterServletFilter.java": [ +42 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/plugins/UpdateCenterClient.java": [ +64 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/user/SecurityRealmFactory.java": [ +38 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/util/ObjectInputStreamIterator.java": [ +31 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/util/cache/DiskCache.java": [ +37 +] +} From ea06a0fcfffcdb8e85db2cca1a41d07975fa6eeb Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Fri, 21 Aug 2026 10:28:55 +0200 Subject: [PATCH 03/18] SONARJAVA-6783: Fix compilation error, add Sonar way profile, and withoutSemantic test - Make inner classes static in FinalizerAttackCheckSample to fix "non-static variable this cannot be referenced from a static context" compilation error caused by FactoryService's static factory method - Add S9345 placeholder to Sonar way quality profile - Add withoutSemantic test since the check only uses syntactic analysis Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 32 +++++++++---------- .../java/checks/FinalizerAttackCheckTest.java | 9 ++++++ .../main/resources/profiles/Sonar_way/S9345 | 0 3 files changed, 25 insertions(+), 16 deletions(-) create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9345 diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index 02377215ec0..a8077a295e7 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -4,7 +4,7 @@ class FinalizerAttackCheckSample { // --- Noncompliant: non-final class with throwing constructor --- - class SecurityService { // Noncompliant {{Make this class "final" or make the throwing constructors "private".}} + static class SecurityService { // Noncompliant {{Make this class "final" or make the throwing constructors "private".}} private final String token; public SecurityService(String token) throws IllegalArgumentException { @@ -15,7 +15,7 @@ public SecurityService(String token) throws IllegalArgumentException { } } - class AuthProvider { // Noncompliant + static class AuthProvider { // Noncompliant public AuthProvider(String credentials) throws Exception { if (credentials.isEmpty()) { throw new Exception("Bad credentials"); @@ -23,7 +23,7 @@ public AuthProvider(String credentials) throws Exception { } } - class ResourceLoader { // Noncompliant + static class ResourceLoader { // Noncompliant ResourceLoader(String path) { if (path == null) { throw new NullPointerException(); @@ -31,7 +31,7 @@ class ResourceLoader { // Noncompliant } } - class MultiConstructorService { // Noncompliant + static class MultiConstructorService { // Noncompliant MultiConstructorService(int id) throws Exception { if (id < 0) { throw new Exception("Negative id"); @@ -42,7 +42,7 @@ class MultiConstructorService { // Noncompliant } } - class ProtectedConstructorService { // Noncompliant + static class ProtectedConstructorService { // Noncompliant protected ProtectedConstructorService(String data) throws Exception { if (data == null) { throw new Exception("Null data"); @@ -50,14 +50,14 @@ protected ProtectedConstructorService(String data) throws Exception { } } - class ThrowsClauseOnly { // Noncompliant + static class ThrowsClauseOnly { // Noncompliant public ThrowsClauseOnly() throws Exception { } } // --- Compliant: final class --- - final class SecureService { + static final class SecureService { public SecureService(String token) throws IllegalArgumentException { if (token == null) { throw new IllegalArgumentException("Invalid token"); @@ -67,7 +67,7 @@ public SecureService(String token) throws IllegalArgumentException { // --- Compliant: all constructors private (factory pattern) --- - class FactoryService { + static class FactoryService { private FactoryService(String data) { } @@ -81,20 +81,20 @@ public static FactoryService create(String data) throws Exception { // --- Compliant: no throwing constructor --- - class SafeService { + static class SafeService { public SafeService(String data) { // no throw } } - class NoConstructor { + static class NoConstructor { void doSomething() { } } // --- Compliant: abstract class --- - abstract class AbstractService { + static abstract class AbstractService { public AbstractService(String data) throws Exception { if (data == null) { throw new Exception("Null"); @@ -104,7 +104,7 @@ public AbstractService(String data) throws Exception { // --- Compliant: private throwing constructor, public non-throwing constructor --- - class MixedConstructors { + static class MixedConstructors { private MixedConstructors(String data) throws Exception { if (data == null) { throw new Exception("Null"); @@ -142,7 +142,7 @@ interface Service { // --- Noncompliant: throw in constructor body without throws clause --- - class ConfigLoader { // Noncompliant + static class ConfigLoader { // Noncompliant public ConfigLoader(String config) { if (config == null) { throw new IllegalStateException("Missing config"); @@ -152,7 +152,7 @@ public ConfigLoader(String config) { // --- Compliant: throw in a method, not in constructor --- - class Processor { + static class Processor { public Processor() { } @@ -163,7 +163,7 @@ public void process() { // --- Noncompliant: nested throw in try block within constructor --- - class DatabaseConnection { // Noncompliant + static class DatabaseConnection { // Noncompliant public DatabaseConnection(String url) { try { if (url == null) { @@ -177,7 +177,7 @@ public DatabaseConnection(String url) { // --- Compliant: all throwing constructors are private --- - class PrivateOnlyThrowers { + static class PrivateOnlyThrowers { private PrivateOnlyThrowers(String s) throws Exception { throw new Exception(); } diff --git a/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java index fb9fbd09c12..8ab83cb9003 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java @@ -31,4 +31,13 @@ void test() { .verifyIssues(); } + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/FinalizerAttackCheckSample.java")) + .withCheck(new FinalizerAttackCheck()) + .withoutSemantic() + .verifyIssues(); + } + } diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9345 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9345 new file mode 100644 index 00000000000..e69de29bb2d From b04aa3b66eae9d729b5129196005e799107570cd Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Fri, 21 Aug 2026 14:39:21 +0200 Subject: [PATCH 04/18] SONARJAVA-6783: Move primary location to throwing constructor with class as secondary The main issue location is now on the throwing constructor (primary) with the class declaration as a secondary location, instead of the other way around. Each vulnerable constructor gets its own issue. Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 32 +++++++++---------- .../java/checks/FinalizerAttackCheck.java | 9 ++++-- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index a8077a295e7..bca0864e9a7 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -4,10 +4,10 @@ class FinalizerAttackCheckSample { // --- Noncompliant: non-final class with throwing constructor --- - static class SecurityService { // Noncompliant {{Make this class "final" or make the throwing constructors "private".}} + static class SecurityService { // Secondary {{Non-final class}} private final String token; - public SecurityService(String token) throws IllegalArgumentException { + public SecurityService(String token) throws IllegalArgumentException { // Noncompliant {{Make this class "final" or make this throwing constructor "private".}} if (token == null) { throw new IllegalArgumentException("Invalid token"); } @@ -15,24 +15,24 @@ public SecurityService(String token) throws IllegalArgumentException { } } - static class AuthProvider { // Noncompliant - public AuthProvider(String credentials) throws Exception { + static class AuthProvider { // Secondary {{Non-final class}} + public AuthProvider(String credentials) throws Exception { // Noncompliant if (credentials.isEmpty()) { throw new Exception("Bad credentials"); } } } - static class ResourceLoader { // Noncompliant - ResourceLoader(String path) { + static class ResourceLoader { // Secondary {{Non-final class}} + ResourceLoader(String path) { // Noncompliant if (path == null) { throw new NullPointerException(); } } } - static class MultiConstructorService { // Noncompliant - MultiConstructorService(int id) throws Exception { + static class MultiConstructorService { // Secondary {{Non-final class}} + MultiConstructorService(int id) throws Exception { // Noncompliant if (id < 0) { throw new Exception("Negative id"); } @@ -42,16 +42,16 @@ static class MultiConstructorService { // Noncompliant } } - static class ProtectedConstructorService { // Noncompliant - protected ProtectedConstructorService(String data) throws Exception { + static class ProtectedConstructorService { // Secondary {{Non-final class}} + protected ProtectedConstructorService(String data) throws Exception { // Noncompliant if (data == null) { throw new Exception("Null data"); } } } - static class ThrowsClauseOnly { // Noncompliant - public ThrowsClauseOnly() throws Exception { + static class ThrowsClauseOnly { // Secondary {{Non-final class}} + public ThrowsClauseOnly() throws Exception { // Noncompliant } } @@ -142,8 +142,8 @@ interface Service { // --- Noncompliant: throw in constructor body without throws clause --- - static class ConfigLoader { // Noncompliant - public ConfigLoader(String config) { + static class ConfigLoader { // Secondary {{Non-final class}} + public ConfigLoader(String config) { // Noncompliant if (config == null) { throw new IllegalStateException("Missing config"); } @@ -163,8 +163,8 @@ public void process() { // --- Noncompliant: nested throw in try block within constructor --- - static class DatabaseConnection { // Noncompliant - public DatabaseConnection(String url) { + static class DatabaseConnection { // Secondary {{Non-final class}} + public DatabaseConnection(String url) { // Noncompliant try { if (url == null) { throw new RuntimeException("Null URL"); diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index 3300f4929b9..862189f4d6c 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -21,6 +21,7 @@ import org.sonar.check.Rule; import org.sonar.java.model.ModifiersUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaFileScannerContext; import org.sonar.plugins.java.api.tree.BaseTreeVisitor; import org.sonar.plugins.java.api.tree.BlockTree; import org.sonar.plugins.java.api.tree.ClassTree; @@ -46,10 +47,14 @@ public void visitNode(Tree tree) { ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.ABSTRACT)) { return; } + List secondaryLocations = Collections.singletonList( + new JavaFileScannerContext.Location("Non-final class", classTree.simpleName())); for (Tree member : classTree.members()) { if (member.is(Kind.CONSTRUCTOR) && isVulnerableConstructor((MethodTree) member)) { - reportIssue(classTree.simpleName(), "Make this class \"final\" or make the throwing constructors \"private\"."); - return; + MethodTree constructor = (MethodTree) member; + reportIssue(constructor.simpleName(), + "Make this class \"final\" or make this throwing constructor \"private\".", + secondaryLocations, null); } } } From 210ab688c0821d6648402a3ba6f9495f62c88205 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 10:03:59 +0200 Subject: [PATCH 05/18] SONARJAVA-6783: Fix NPE on anonymous classes, skip sealed classes, update rulings - Add null check for classTree.simpleName() to prevent NPE on anonymous classes - Skip sealed classes since they cannot be subclassed by attackers - Add sealed class test case - Add ruling results for eclipse-jetty-similar-to-main Co-Authored-By: Claude Opus 4.6 --- .../java-S9345.json | 35 +++++++++++++++++++ .../checks/FinalizerAttackCheckSample.java | 16 +++++++++ .../java/checks/FinalizerAttackCheck.java | 6 ++-- 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json new file mode 100644 index 00000000000..de4800cd0ee --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json @@ -0,0 +1,35 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java": [ +28 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ +32 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java": [ +30 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java": [ +41 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java": [ +39 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java": [ +273 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java": [ +41 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java": [ +29 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java": [ +33 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java": [ +83 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java": [ +38 +] +} diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index bca0864e9a7..809053282fd 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -134,6 +134,22 @@ record Credential(String value) { } } + // --- Compliant: sealed class (cannot be subclassed by attackers) --- + + static sealed class SealedService permits AllowedSubclass { + public SealedService(String data) throws Exception { + if (data == null) { + throw new Exception("Null"); + } + } + } + + static final class AllowedSubclass extends SealedService { + public AllowedSubclass(String data) throws Exception { + super(data); + } + } + // --- Compliant: inner interface (no constructors) --- interface Service { diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index 862189f4d6c..1166707e0ba 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -43,8 +43,10 @@ public List nodesToVisit() { @Override public void visitNode(Tree tree) { ClassTree classTree = (ClassTree) tree; - if (ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.FINAL) || - ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.ABSTRACT)) { + if (classTree.simpleName() == null || + ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.FINAL) || + ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.ABSTRACT) || + ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.SEALED)) { return; } List secondaryLocations = Collections.singletonList( From f58ad990a95652cfaf877efa794971d28c21f919 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:29:50 +0100 Subject: [PATCH 06/18] Update ruling results for PR #5981 (#5983) Co-authored-by: github-actions[bot] --- .../commons-beanutils/java-S9345.json | 29 ++++--- .../resources/eclipse-jetty/java-S9345.json | 87 +++++++++++-------- .../src/test/resources/guava/java-S9345.json | 4 +- .../resources/sonar-server/java-S9345.json | 14 +-- 4 files changed, 78 insertions(+), 56 deletions(-) diff --git a/its/ruling/src/test/resources/commons-beanutils/java-S9345.json b/its/ruling/src/test/resources/commons-beanutils/java-S9345.json index efeec87b4a1..88ebe3ed5fd 100644 --- a/its/ruling/src/test/resources/commons-beanutils/java-S9345.json +++ b/its/ruling/src/test/resources/commons-beanutils/java-S9345.json @@ -1,32 +1,41 @@ { "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanPropertyValueChangeClosure.java": [ -79 +134 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanPropertyValueEqualsPredicate.java": [ -110 +164 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanToPropertyValueTransformer.java": [ -71 +119 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/FluentPropertyBeanIntrospector.java": [ -78 +97 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java": [ -44 +85, +151, +197 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/MethodUtils.java": [ -1304 +1319 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/ResultSetDynaClass.java": [ -82 +100, +128, +159 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/RowSetDynaClass.java": [ -66 +101, +123, +148, +176, +206, +236 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/SuppressPropertiesBeanIntrospector.java": [ -38 +62 ], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/converters/ArrayConverter.java": [ -129 +150 ] } diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json index 2ef088f3283..3fcaf3c72b6 100644 --- a/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json @@ -1,107 +1,120 @@ { "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java": [ -28 +37 ], "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ -32 +125 ], "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java": [ -30 +36 ], "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java": [ -41 +76 ], "org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java": [ -39 +90 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java": [ -273 +303 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java": [ -41 +68 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java": [ -29 +33 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java": [ -33 +54 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java": [ -83 +370 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java": [ -38 +47 ], "org.eclipse.jetty:jetty-project:jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONPojoConvertorFactory.java": [ -29 +46 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/BlockingArrayQueue.java": [ -49 +126 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ClassLoadingObjectInputStream.java": [ -32 +48, +53 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/CountingCallback.java": [ -41 +45 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java": [ -26 +37 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java": [ -39 +85 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/InetAddressPattern.java": [ -110, -191, -236 +115, +198, +241 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java": [ -29 +43, +53 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartWriter.java": [ -28 +41 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java": [ -38, -154 +55, +68, +162 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/PathWatcher.java": [ -70 +98 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java": [ -37 +52 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/RolloverFileOutputStream.java": [ -51 +79, +91, +104, +120, +140, +151 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/Uptime.java": [ -36 +41 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/component/FileDestroyable.java": [ -32 +41 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java": [ -53 +279, +331 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java": [ -43 +89 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/security/CertificateValidator.java": [ -55 +86 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java": [ -40 +48 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/X509.java": [ -37 +67 ], "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java": [ -48 +126 ], "org.eclipse.jetty:jetty-project:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlAppendable.java": [ -30 +38, +43, +48, +53, +58 ], "org.eclipse.jetty:jetty-project:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java": [ -87 +224 ] } diff --git a/its/ruling/src/test/resources/guava/java-S9345.json b/its/ruling/src/test/resources/guava/java-S9345.json index 68013f90b30..483ffcbef99 100644 --- a/its/ruling/src/test/resources/guava/java-S9345.json +++ b/its/ruling/src/test/resources/guava/java-S9345.json @@ -1,8 +1,8 @@ { "com.google.guava:guava:src/com/google/common/base/FinalizableReferenceQueue.java": [ -94 +159 ], "com.google.guava:guava:src/com/google/common/io/MultiReader.java": [ -33 +37 ] } diff --git a/its/ruling/src/test/resources/sonar-server/java-S9345.json b/its/ruling/src/test/resources/sonar-server/java-S9345.json index bcad8a0b67f..1daebf5052b 100644 --- a/its/ruling/src/test/resources/sonar-server/java-S9345.json +++ b/its/ruling/src/test/resources/sonar-server/java-S9345.json @@ -1,23 +1,23 @@ { "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/source/ReportIterator.java": [ -33 +38 ], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java": [ -53 +114 ], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/platform/web/MasterServletFilter.java": [ -42 +48 ], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/plugins/UpdateCenterClient.java": [ -64 +75 ], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/user/SecurityRealmFactory.java": [ -38 +43 ], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/util/ObjectInputStreamIterator.java": [ -31 +35 ], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/util/cache/DiskCache.java": [ -37 +42 ] } From 20b8c8e6d64fe370407703b3d12fdbb3ce437d1e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:49:27 +0100 Subject: [PATCH 07/18] Update ruling results for PR #5981 (#6000) Co-authored-by: github-actions[bot] --- .../java-S9345.json | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json index de4800cd0ee..ca313f2f0f8 100644 --- a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json @@ -1,35 +1,35 @@ { "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java": [ -28 +37 ], "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ -32 +125 ], "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java": [ -30 +36 ], "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java": [ -41 +76 ], "org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java": [ -39 +90 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java": [ -273 +303 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java": [ -41 +68 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java": [ -29 +33 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java": [ -33 +54 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java": [ -83 +370 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java": [ -38 +47 ] } From 09126f54a11d22a9c224f824ba120b0322687860 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 12:23:16 +0200 Subject: [PATCH 08/18] SONARJAVA-6783: Address review feedback for S9345 FinalizerAttackCheck - Remove abstract class exemption (abstract classes can be subclassed) - Refine sealed class logic: only skip sealed classes whose permitted subclasses are all final/sealed (flag those with non-sealed permits) - Recognize final finalize() method as a mitigation (skip these classes) - Detect throwing instance initializers in classes without explicit constructors - Exclude local classes (cannot be subclassed from other files) - Fix CWE mapping: remove incorrect CWE-586 (Explicit Call to Finalize), keep CERT OBJ11-J mapping - Add comprehensive test cases for all new behaviors Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 88 +++++++++- .../java/checks/FinalizerAttackCheck.java | 154 +++++++++++++++++- .../org/sonar/l10n/java/rules/java/S9345.html | 1 - .../org/sonar/l10n/java/rules/java/S9345.json | 5 +- 4 files changed, 227 insertions(+), 21 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index 809053282fd..638e87d40d3 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -92,10 +92,10 @@ void doSomething() { } } - // --- Compliant: abstract class --- + // --- Noncompliant: abstract class with throwing constructor (attacker can subclass) --- - static abstract class AbstractService { - public AbstractService(String data) throws Exception { + static abstract class AbstractService { // Secondary {{Non-final class}} + public AbstractService(String data) throws Exception { // Noncompliant if (data == null) { throw new Exception("Null"); } @@ -134,22 +134,38 @@ record Credential(String value) { } } - // --- Compliant: sealed class (cannot be subclassed by attackers) --- + // --- Compliant: sealed class whose permitted subclasses are all final/sealed --- - static sealed class SealedService permits AllowedSubclass { - public SealedService(String data) throws Exception { + static sealed class SealedServiceAllFinal permits AllowedSubclass { + public SealedServiceAllFinal(String data) throws Exception { if (data == null) { throw new Exception("Null"); } } } - static final class AllowedSubclass extends SealedService { + static final class AllowedSubclass extends SealedServiceAllFinal { public AllowedSubclass(String data) throws Exception { super(data); } } + // --- Noncompliant: sealed class with a non-sealed permitted subclass --- + + static sealed class SealedServiceWithNonSealed permits OpenSubclass { // Secondary {{Non-final class}} + public SealedServiceWithNonSealed(String data) throws Exception { // Noncompliant + if (data == null) { + throw new Exception("Null"); + } + } + } + + static non-sealed class OpenSubclass extends SealedServiceWithNonSealed { // Secondary {{Non-final class}} + public OpenSubclass(String data) throws Exception { // Noncompliant + super(data); + } + } + // --- Compliant: inner interface (no constructors) --- interface Service { @@ -202,4 +218,62 @@ private PrivateOnlyThrowers(int i) { throw new IllegalArgumentException(); } } + + // --- Compliant: class declares final finalize() method --- + + static class ProtectedByFinalizer { + public ProtectedByFinalizer(String data) throws Exception { + if (data == null) { + throw new Exception("Null"); + } + } + + @Override + protected final void finalize() { + // prevents finalizer attack + } + } + + // --- Noncompliant: instance initializer throws, no explicit constructor --- + + static class InitializerThrower { // Noncompliant {{Make this class "final" or add a private constructor, because initializers can throw.}} + { // Secondary {{Throwing initializer}} + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init"); + } + } + } + + // --- Compliant: field initializer calls a method that may throw, but no direct throw statement --- + + static class FieldInitializerMethodCall { + private final Object value = computeValue(); + + private static Object computeValue() { + throw new UnsupportedOperationException(); + } + } + + // --- Compliant: instance initializer throws but has explicit private constructor --- + + static class InitializerWithPrivateConstructor { + { + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init"); + } + } + + private InitializerWithPrivateConstructor() { + } + } + + // --- Compliant: local class (cannot be subclassed from outside) --- + + void someMethod() { + class LocalClass { + LocalClass() throws Exception { + throw new Exception("local"); + } + } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index 1166707e0ba..1661974a5e7 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -16,21 +16,28 @@ */ package org.sonar.java.checks; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import org.sonar.check.Rule; import org.sonar.java.model.ModifiersUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.BaseTreeVisitor; import org.sonar.plugins.java.api.tree.BlockTree; import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.CompilationUnitTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.LambdaExpressionTree; import org.sonar.plugins.java.api.tree.MethodTree; import org.sonar.plugins.java.api.tree.Modifier; import org.sonar.plugins.java.api.tree.ThrowStatementTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.Tree.Kind; +import org.sonar.plugins.java.api.tree.TypeTree; +import org.sonar.plugins.java.api.tree.VariableTree; @Rule(key = "S9345") public class FinalizerAttackCheck extends IssuableSubscriptionVisitor { @@ -45,20 +52,136 @@ public void visitNode(Tree tree) { ClassTree classTree = (ClassTree) tree; if (classTree.simpleName() == null || ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.FINAL) || - ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.ABSTRACT) || - ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.SEALED)) { + isLocalClass(classTree) || + isSafelySealedClass(classTree) || + hasFinalFinalizer(classTree)) { return; } List secondaryLocations = Collections.singletonList( new JavaFileScannerContext.Location("Non-final class", classTree.simpleName())); + + boolean hasExplicitConstructor = false; + List throwingInitializers = new ArrayList<>(); + for (Tree member : classTree.members()) { - if (member.is(Kind.CONSTRUCTOR) && isVulnerableConstructor((MethodTree) member)) { - MethodTree constructor = (MethodTree) member; - reportIssue(constructor.simpleName(), - "Make this class \"final\" or make this throwing constructor \"private\".", - secondaryLocations, null); + if (member.is(Kind.CONSTRUCTOR)) { + hasExplicitConstructor = true; + if (isVulnerableConstructor((MethodTree) member)) { + MethodTree constructor = (MethodTree) member; + reportIssue(constructor.simpleName(), + "Make this class \"final\" or make this throwing constructor \"private\".", + secondaryLocations, null); + } + } else if (member.is(Kind.INITIALIZER) && containsThrowStatementInBlock((BlockTree) member)) { + throwingInitializers.add(member); + } else if (member.is(Kind.VARIABLE) && hasThrowingFieldInitializer((VariableTree) member)) { + throwingInitializers.add(member); + } + } + + if (!hasExplicitConstructor && !throwingInitializers.isEmpty()) { + List locations = new ArrayList<>(); + for (Tree init : throwingInitializers) { + locations.add(new JavaFileScannerContext.Location("Throwing initializer", init)); + } + reportIssue(classTree.simpleName(), + "Make this class \"final\" or add a private constructor, because initializers can throw.", + locations, null); + } + } + + private static boolean isLocalClass(ClassTree classTree) { + Tree parent = classTree.parent(); + while (parent != null) { + if (parent.is(Kind.METHOD, Kind.CONSTRUCTOR)) { + return true; + } + if (parent.is(Kind.CLASS, Kind.ENUM, Kind.INTERFACE, Kind.RECORD, Kind.ANNOTATION_TYPE)) { + return false; + } + parent = parent.parent(); + } + return false; + } + + private static boolean isSafelySealedClass(ClassTree classTree) { + if (!ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.SEALED)) { + return false; + } + for (TypeTree permitted : classTree.permittedTypes()) { + Type permittedType = permitted.symbolType(); + if (!permittedType.isUnknown()) { + Symbol.TypeSymbol permittedSymbol = permittedType.symbol(); + ClassTree permittedDecl = permittedSymbol.declaration(); + if (permittedDecl != null && ModifiersUtils.hasModifier(permittedDecl.modifiers(), Modifier.NON_SEALED)) { + return false; + } + } else { + ClassTree permittedDecl = findClassByName(classTree, getSimpleName(permitted)); + if (permittedDecl != null && ModifiersUtils.hasModifier(permittedDecl.modifiers(), Modifier.NON_SEALED)) { + return false; + } } } + return true; + } + + private static String getSimpleName(TypeTree typeTree) { + if (typeTree.is(Kind.IDENTIFIER)) { + return ((IdentifierTree) typeTree).name(); + } + return ""; + } + + private static ClassTree findClassByName(ClassTree context, String name) { + if (name.isEmpty()) { + return null; + } + Tree parent = context.parent(); + while (parent != null && !parent.is(Kind.COMPILATION_UNIT)) { + parent = parent.parent(); + } + if (parent == null) { + return null; + } + return findClassInTree(parent, name); + } + + private static ClassTree findClassInTree(Tree tree, String name) { + if (tree.is(Kind.CLASS, Kind.INTERFACE)) { + ClassTree classTree = (ClassTree) tree; + if (classTree.simpleName() != null && name.equals(classTree.simpleName().name())) { + return classTree; + } + for (Tree member : classTree.members()) { + ClassTree found = findClassInTree(member, name); + if (found != null) { + return found; + } + } + } else if (tree.is(Kind.COMPILATION_UNIT)) { + for (Tree child : ((CompilationUnitTree) tree).types()) { + ClassTree found = findClassInTree(child, name); + if (found != null) { + return found; + } + } + } + return null; + } + + private static boolean hasFinalFinalizer(ClassTree classTree) { + for (Tree member : classTree.members()) { + if (member.is(Kind.METHOD)) { + MethodTree method = (MethodTree) member; + if ("finalize".equals(method.simpleName().name()) && + method.parameters().isEmpty() && + ModifiersUtils.hasModifier(method.modifiers(), Modifier.FINAL)) { + return true; + } + } + } + return false; } private static boolean isVulnerableConstructor(MethodTree constructor) { @@ -68,16 +191,29 @@ private static boolean isVulnerableConstructor(MethodTree constructor) { return !constructor.throwsClauses().isEmpty() || containsThrowStatement(constructor); } - private static boolean containsThrowStatement(MethodTree constructor) { - BlockTree block = constructor.block(); + private static boolean containsThrowStatement(MethodTree method) { + BlockTree block = method.block(); if (block == null) { return false; } + return containsThrowStatementInBlock(block); + } + + private static boolean containsThrowStatementInBlock(BlockTree block) { ThrowStatementVisitor visitor = new ThrowStatementVisitor(); block.accept(visitor); return visitor.hasThrow; } + private static boolean hasThrowingFieldInitializer(VariableTree variable) { + if (variable.initializer() == null) { + return false; + } + ThrowStatementVisitor visitor = new ThrowStatementVisitor(); + variable.initializer().accept(visitor); + return visitor.hasThrow; + } + private static class ThrowStatementVisitor extends BaseTreeVisitor { boolean hasThrow; diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html index ff87bbc6281..3ee4fee68ae 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html @@ -62,7 +62,6 @@

Documentation

Standards

diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json index 87c32a9e4cf..5712e011867 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json @@ -7,9 +7,7 @@ "constantCost": "5min" }, "tags": [ - "cert", - "cwe", - "serialization" + "cert" ], "defaultSeverity": "Critical", "ruleSpecification": "RSPEC-9345", @@ -26,7 +24,6 @@ "attribute": "COMPLETE" }, "securityStandards": { - "CWE": [586], "CERT": ["OBJ11-J."] } } From f0c1680fd0ed18d3d35ccc14c23aaecd02c98865 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 12:45:40 +0200 Subject: [PATCH 09/18] SONARJAVA-6783: Reduce cognitive complexity and add test coverage for S9345 Extract checkMembers, reportVulnerableConstructor, and reportThrowingInitializers helper methods from visitNode to reduce cognitive complexity below the threshold. Add test cases for: non-final finalize(), throws in lambdas/anonymous classes, multiple throwing initializers, local class in constructor, and field initializers. Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 90 +++++++++++++++++++ .../java/checks/FinalizerAttackCheck.java | 37 +++++--- 2 files changed, 114 insertions(+), 13 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index 638e87d40d3..93402401e49 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -276,4 +276,94 @@ class LocalClass { } } } + + // --- Compliant: field initializer calls a method, no direct throw in initializer expression --- + + static class FieldInitializerIndirectThrow { + private final Object data = check(null); + + private static Object check(Object o) { + if (o == null) { + throw new IllegalArgumentException(); + } + return o; + } + } + + // --- Noncompliant: non-final finalize() does not protect --- + + static class NonFinalFinalize { // Secondary {{Non-final class}} + public NonFinalFinalize(String data) throws Exception { // Noncompliant + if (data == null) { + throw new Exception("Null"); + } + } + + @Override + protected void finalize() { + // non-final finalize does NOT protect + } + } + + // --- Compliant: throw only inside lambda in constructor --- + + static class ThrowInLambda { + public ThrowInLambda() { + Runnable r = () -> { + throw new RuntimeException("in lambda"); + }; + } + } + + // --- Compliant: throw only inside anonymous class in constructor --- + + static class ThrowInAnonymousClass { + public ThrowInAnonymousClass() { + Runnable r = new Runnable() { + @Override + public void run() { + throw new RuntimeException("in anon"); + } + }; + } + } + + // --- Noncompliant: multiple throwing instance initializers, no explicit constructor --- + + static class MultipleThrowingInitializers { // Noncompliant {{Make this class "final" or add a private constructor, because initializers can throw.}} + { // Secondary {{Throwing initializer}} + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init block 1"); + } + } + { // Secondary {{Throwing initializer}} + if (System.currentTimeMillis() == 1) { + throw new RuntimeException("init block 2"); + } + } + } + + // --- Compliant: local class inside a constructor --- + + static class EnclosingWithLocalInConstructor { + EnclosingWithLocalInConstructor() { + class InnerLocal { + InnerLocal() throws Exception { + throw new Exception("local in ctor"); + } + } + } + } + + // --- Compliant: field initializer without throw --- + + static class FieldInitializerNoThrow { + private final String data = "hello"; + } + + // --- Compliant: field without initializer --- + + static class FieldNoInitializer { + private String data; + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index 1661974a5e7..da38005553e 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -60,18 +60,17 @@ public void visitNode(Tree tree) { List secondaryLocations = Collections.singletonList( new JavaFileScannerContext.Location("Non-final class", classTree.simpleName())); + checkMembers(classTree, secondaryLocations); + } + + private void checkMembers(ClassTree classTree, List secondaryLocations) { boolean hasExplicitConstructor = false; List throwingInitializers = new ArrayList<>(); for (Tree member : classTree.members()) { if (member.is(Kind.CONSTRUCTOR)) { hasExplicitConstructor = true; - if (isVulnerableConstructor((MethodTree) member)) { - MethodTree constructor = (MethodTree) member; - reportIssue(constructor.simpleName(), - "Make this class \"final\" or make this throwing constructor \"private\".", - secondaryLocations, null); - } + reportVulnerableConstructor((MethodTree) member, secondaryLocations); } else if (member.is(Kind.INITIALIZER) && containsThrowStatementInBlock((BlockTree) member)) { throwingInitializers.add(member); } else if (member.is(Kind.VARIABLE) && hasThrowingFieldInitializer((VariableTree) member)) { @@ -80,14 +79,26 @@ public void visitNode(Tree tree) { } if (!hasExplicitConstructor && !throwingInitializers.isEmpty()) { - List locations = new ArrayList<>(); - for (Tree init : throwingInitializers) { - locations.add(new JavaFileScannerContext.Location("Throwing initializer", init)); - } - reportIssue(classTree.simpleName(), - "Make this class \"final\" or add a private constructor, because initializers can throw.", - locations, null); + reportThrowingInitializers(classTree, throwingInitializers); + } + } + + private void reportVulnerableConstructor(MethodTree constructor, List secondaryLocations) { + if (isVulnerableConstructor(constructor)) { + reportIssue(constructor.simpleName(), + "Make this class \"final\" or make this throwing constructor \"private\".", + secondaryLocations, null); + } + } + + private void reportThrowingInitializers(ClassTree classTree, List throwingInitializers) { + List locations = new ArrayList<>(); + for (Tree init : throwingInitializers) { + locations.add(new JavaFileScannerContext.Location("Throwing initializer", init)); } + reportIssue(classTree.simpleName(), + "Make this class \"final\" or add a private constructor, because initializers can throw.", + locations, null); } private static boolean isLocalClass(ClassTree classTree) { From 9cbfead658ff5efe9967a00388e9ab8a16b09c51 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 11:54:24 +0100 Subject: [PATCH 10/18] Update ruling results for PR #5981 (#6007) Co-authored-by: github-actions[bot] --- .../src/test/resources/commons-beanutils/java-S9345.json | 3 +++ its/ruling/src/test/resources/eclipse-jetty/java-S9345.json | 3 +++ 2 files changed, 6 insertions(+) diff --git a/its/ruling/src/test/resources/commons-beanutils/java-S9345.json b/its/ruling/src/test/resources/commons-beanutils/java-S9345.json index 88ebe3ed5fd..965fe05a3d5 100644 --- a/its/ruling/src/test/resources/commons-beanutils/java-S9345.json +++ b/its/ruling/src/test/resources/commons-beanutils/java-S9345.json @@ -1,4 +1,7 @@ { +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BaseDynaBeanMapDecorator.java": [ +78 +], "commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanPropertyValueChangeClosure.java": [ 134 ], diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json index 3fcaf3c72b6..f183c827e6d 100644 --- a/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json @@ -11,6 +11,9 @@ "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java": [ 76 ], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java": [ +51 +], "org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java": [ 90 ], From e98ae924bbf56c8a494cc5888128b2683377686b Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 12:59:33 +0200 Subject: [PATCH 11/18] SONARJAVA-6783: Flag constructors when class has throwing initializers Non-private constructors are now reported even if they don't throw themselves, when the class has instance initializer blocks that throw. Instance initializers run as part of every constructor, so a throwing initializer makes all non-private constructors vulnerable to finalizer attacks. Also adds test cases for: abstract classes with throwing initializers, deep sealed hierarchies, final finalize() with initializers, static initializers (compliant), and constructors combined with throwing initializers. Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 87 +++++++++++++++++++ .../java/checks/FinalizerAttackCheck.java | 28 ++++-- 2 files changed, 106 insertions(+), 9 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index 93402401e49..a51949ac982 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -366,4 +366,91 @@ static class FieldInitializerNoThrow { static class FieldNoInitializer { private String data; } + + // --- Noncompliant: explicit non-private constructor AND throwing initializer --- + + static class ConstructorAndThrowingInitializer { // Secondary {{Non-final class}} + { + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init"); + } + } + + public ConstructorAndThrowingInitializer(String data) throws Exception { // Noncompliant + if (data == null) { + throw new Exception("Null"); + } + } + } + + // --- Noncompliant: explicit non-throwing constructor AND throwing initializer (constructor is still a vector) --- + + static class NonThrowingConstructorWithThrowingInit { // Secondary {{Non-final class}} + { + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init"); + } + } + + public NonThrowingConstructorWithThrowingInit() { // Noncompliant + // non-throwing, but the initializer block throws during construction + } + } + + // --- Noncompliant: abstract class with throwing initializer, no constructor --- + + static abstract class AbstractWithThrowingInitializer { // Noncompliant {{Make this class "final" or add a private constructor, because initializers can throw.}} + { // Secondary {{Throwing initializer}} + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("abstract init"); + } + } + } + + // --- Compliant: sealed class with all final + sealed subclasses (deep hierarchy) --- + + static sealed class DeepSealedParent permits DeepSealedChild { + public DeepSealedParent(String data) throws Exception { + if (data == null) { + throw new Exception("Null"); + } + } + } + + static sealed class DeepSealedChild extends DeepSealedParent permits DeepSealedGrandchild { + public DeepSealedChild(String data) throws Exception { + super(data); + } + } + + static final class DeepSealedGrandchild extends DeepSealedChild { + public DeepSealedGrandchild(String data) throws Exception { + super(data); + } + } + + // --- Compliant: class with final finalize() and throwing initializer --- + + static class FinalFinalizerWithThrowingInit { + { + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init"); + } + } + + @Override + protected final void finalize() { + // prevents finalizer attack + } + } + + // --- Compliant: class with only static initializer that throws --- + + static class StaticInitializerThrower { + static { + if (System.getenv("MISSING") == null) { + throw new RuntimeException("static init"); + } + } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index da38005553e..0eeff0f9813 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -65,29 +65,39 @@ public void visitNode(Tree tree) { private void checkMembers(ClassTree classTree, List secondaryLocations) { boolean hasExplicitConstructor = false; + boolean hasThrowingInitializers = false; List throwingInitializers = new ArrayList<>(); for (Tree member : classTree.members()) { if (member.is(Kind.CONSTRUCTOR)) { hasExplicitConstructor = true; - reportVulnerableConstructor((MethodTree) member, secondaryLocations); } else if (member.is(Kind.INITIALIZER) && containsThrowStatementInBlock((BlockTree) member)) { throwingInitializers.add(member); + hasThrowingInitializers = true; } else if (member.is(Kind.VARIABLE) && hasThrowingFieldInitializer((VariableTree) member)) { throwingInitializers.add(member); + hasThrowingInitializers = true; } } - if (!hasExplicitConstructor && !throwingInitializers.isEmpty()) { + if (hasExplicitConstructor) { + reportVulnerableConstructors(classTree, hasThrowingInitializers, secondaryLocations); + } else if (hasThrowingInitializers) { reportThrowingInitializers(classTree, throwingInitializers); } } - private void reportVulnerableConstructor(MethodTree constructor, List secondaryLocations) { - if (isVulnerableConstructor(constructor)) { - reportIssue(constructor.simpleName(), - "Make this class \"final\" or make this throwing constructor \"private\".", - secondaryLocations, null); + private void reportVulnerableConstructors(ClassTree classTree, boolean hasThrowingInitializers, + List secondaryLocations) { + for (Tree member : classTree.members()) { + if (member.is(Kind.CONSTRUCTOR)) { + MethodTree constructor = (MethodTree) member; + if (isVulnerableConstructor(constructor, hasThrowingInitializers)) { + reportIssue(constructor.simpleName(), + "Make this class \"final\" or make this throwing constructor \"private\".", + secondaryLocations, null); + } + } } } @@ -195,11 +205,11 @@ private static boolean hasFinalFinalizer(ClassTree classTree) { return false; } - private static boolean isVulnerableConstructor(MethodTree constructor) { + private static boolean isVulnerableConstructor(MethodTree constructor, boolean hasThrowingInitializers) { if (ModifiersUtils.hasModifier(constructor.modifiers(), Modifier.PRIVATE)) { return false; } - return !constructor.throwsClauses().isEmpty() || containsThrowStatement(constructor); + return hasThrowingInitializers || !constructor.throwsClauses().isEmpty() || containsThrowStatement(constructor); } private static boolean containsThrowStatement(MethodTree method) { From 6630fc76c8c3e7a2ced8731f0d9ad32dcb318680 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 24 Aug 2026 11:15:51 +0000 Subject: [PATCH 12/18] Update ruling results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with GitHub Actions --- .../resources/eclipse-jetty-similar-to-main/java-S9345.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json index ca313f2f0f8..ecb785c6447 100644 --- a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json @@ -11,6 +11,9 @@ "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java": [ 76 ], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java": [ +51 +], "org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java": [ 90 ], From de2188c2a35814265cad2fd3e847e9c06b46aa77 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 13:20:36 +0200 Subject: [PATCH 13/18] Update ruling results for eclipse-jetty-similar-to-main S9345 Co-Authored-By: Claude Opus 4.6 --- .../resources/eclipse-jetty-similar-to-main/java-S9345.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json index ca313f2f0f8..ecb785c6447 100644 --- a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json @@ -11,6 +11,9 @@ "org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java": [ 76 ], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java": [ +51 +], "org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java": [ 90 ], From a3aeff0cf160c0592af4a8f56d737a011b8e0795 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 13:33:19 +0200 Subject: [PATCH 14/18] SONARJAVA-6783: Fix duplicate branch code, reduce complexity, and add test coverage for S9345 Extract sealed class resolution logic into separate methods to eliminate duplicate branch code (S1871) and reduce cognitive complexity (S3776). Add additional test cases for coverage. Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 60 +++++++++++++++++++ .../java/checks/FinalizerAttackCheck.java | 27 +++++---- 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index a51949ac982..5805f196cd3 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -453,4 +453,64 @@ static class StaticInitializerThrower { } } } + + // --- Compliant: field initializer calls method (no direct throw in initializer) --- + + static class FieldInitializerIndirectThrow2 { + private final Object value = throwingInit(); + + private static Object throwingInit() { + throw new UnsupportedOperationException(); + } + } + + // --- Compliant: field initializer is a method call (no direct throw in initializer expression) --- + + static class FieldInitWithExplicitConstructor { + private final Object data = initField(); + + public FieldInitWithExplicitConstructor() { + } + + private Object initField() { + throw new UnsupportedOperationException(); + } + } + + // --- Compliant: abstract class with non-throwing constructor and no initializers --- + + static abstract class AbstractNoThrow { + public AbstractNoThrow() { + } + } + + // --- Compliant: sealed class with only sealed/final subclasses (resolved via symbolType) --- + + static sealed class SealedResolved permits ResolvedFinalChild { + public SealedResolved(String s) throws Exception { + if (s == null) throw new Exception(); + } + } + + static final class ResolvedFinalChild extends SealedResolved { + ResolvedFinalChild(String s) throws Exception { + super(s); + } + } + + // --- Noncompliant: class with multiple constructors, some throwing --- + + static class PartiallyVulnerable { // Secondary {{Non-final class}} + PartiallyVulnerable(int x) throws Exception { // Noncompliant + if (x < 0) throw new Exception(); + } + + private PartiallyVulnerable(String s) throws Exception { + if (s == null) throw new Exception(); + } + + PartiallyVulnerable(double d) { + // compliant: non-throwing + } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index 0eeff0f9813..fd1305fa170 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -130,23 +130,26 @@ private static boolean isSafelySealedClass(ClassTree classTree) { return false; } for (TypeTree permitted : classTree.permittedTypes()) { - Type permittedType = permitted.symbolType(); - if (!permittedType.isUnknown()) { - Symbol.TypeSymbol permittedSymbol = permittedType.symbol(); - ClassTree permittedDecl = permittedSymbol.declaration(); - if (permittedDecl != null && ModifiersUtils.hasModifier(permittedDecl.modifiers(), Modifier.NON_SEALED)) { - return false; - } - } else { - ClassTree permittedDecl = findClassByName(classTree, getSimpleName(permitted)); - if (permittedDecl != null && ModifiersUtils.hasModifier(permittedDecl.modifiers(), Modifier.NON_SEALED)) { - return false; - } + if (isNonSealedPermittedType(classTree, permitted)) { + return false; } } return true; } + private static boolean isNonSealedPermittedType(ClassTree context, TypeTree permitted) { + ClassTree permittedDecl = resolvePermittedDeclaration(context, permitted); + return permittedDecl != null && ModifiersUtils.hasModifier(permittedDecl.modifiers(), Modifier.NON_SEALED); + } + + private static ClassTree resolvePermittedDeclaration(ClassTree context, TypeTree permitted) { + Type permittedType = permitted.symbolType(); + if (!permittedType.isUnknown()) { + return permittedType.symbol().declaration(); + } + return findClassByName(context, getSimpleName(permitted)); + } + private static String getSimpleName(TypeTree typeTree) { if (typeTree.is(Kind.IDENTIFIER)) { return ((IdentifierTree) typeTree).name(); From 695ec01b4eae8fef16b810a9a17f8b7f61aea4e8 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 13:47:04 +0200 Subject: [PATCH 15/18] SONARJAVA-6783: Add test coverage and merge ruling fix for S9345 - Add test cases for abstract classes, sealed class hierarchies, finalize signature edge cases, and field initializer edge cases - Merge ruling fix PR #6010 (eclipse-jetty-similar-to-main) Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index 5805f196cd3..ae6fcb63d5e 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -513,4 +513,102 @@ private PartiallyVulnerable(String s) throws Exception { // compliant: non-throwing } } + + // --- Compliant: abstract class with no throwing constructor and no throwing initializer --- + + static abstract class AbstractSafeClass { + public AbstractSafeClass(String data) { + // no throw + } + + abstract void doWork(); + } + + // --- Noncompliant: abstract class with constructor that has throws clause only --- + + static abstract class AbstractThrowsClause { // Secondary {{Non-final class}} + protected AbstractThrowsClause() throws Exception { // Noncompliant + } + } + + // --- Compliant: sealed class with sealed child (not non-sealed) --- + + static sealed class SealedWithSealedChild permits SealedChild { + public SealedWithSealedChild(String s) throws Exception { + if (s == null) throw new Exception(); + } + } + + static sealed class SealedChild extends SealedWithSealedChild permits FinalGrandchild { + public SealedChild(String s) throws Exception { + super(s); + } + } + + static final class FinalGrandchild extends SealedChild { + public FinalGrandchild(String s) throws Exception { + super(s); + } + } + + // --- Compliant: abstract class with only abstract methods --- + + static abstract class AbstractMethodOnly { + abstract void compute(); + } + + // --- Compliant: field initializer without direct throw --- + + static class FieldInitializerSafe { + private final String value = String.valueOf(42); + + public FieldInitializerSafe() { + } + } + + // --- Compliant: field initializer is anonymous class with throw (skipped by visitor) --- + + static class FieldInitAnonymousThrow { + private final Runnable action = new Runnable() { + @Override + public void run() { + throw new RuntimeException("in anon in field init"); + } + }; + } + + // --- Noncompliant: non-final, non-private constructors where one has throw and one has throws clause --- + + static class BothThrowAndThrowsClause { // Secondary {{Non-final class}} + public BothThrowAndThrowsClause(int x) { // Noncompliant + if (x < 0) { + throw new IllegalArgumentException(); + } + } + + protected BothThrowAndThrowsClause(String s) throws Exception { // Noncompliant + } + } + + // --- Noncompliant: finalize(Object) is not zero-arg finalize, does not protect --- + + static class WrongFinalizeSignature { // Secondary {{Non-final class}} + public WrongFinalizeSignature(String s) throws Exception { // Noncompliant + if (s == null) throw new Exception(); + } + + protected final void finalize(Object obj) { + // wrong signature, does not protect + } + } + + // --- Noncompliant: class with field without initializer but throwing constructor --- + + static class FieldWithoutInitializer { // Secondary {{Non-final class}} + private Object data; + + public FieldWithoutInitializer() throws Exception { // Noncompliant + throw new Exception(); + } + } } From d03d53552fc22cbc0811806f0abc2b9f7438ce8a Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 13:58:58 +0200 Subject: [PATCH 16/18] SONARJAVA-6783: Remove unused Symbol import in FinalizerAttackCheck Co-Authored-By: Claude Opus 4.6 --- .../main/java/org/sonar/java/checks/FinalizerAttackCheck.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index fd1305fa170..7129644336c 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -23,7 +23,6 @@ import org.sonar.java.model.ModifiersUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.JavaFileScannerContext; -import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.BaseTreeVisitor; import org.sonar.plugins.java.api.tree.BlockTree; From a4994f55145677815d0dccb2c8e2bf59beb29a72 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 14:50:27 +0200 Subject: [PATCH 17/18] SONARJAVA-6783: Reduce complexity, add test coverage for S9345 Extract findClassInChildren helper to reduce cognitive complexity of findClassInTree. Add non-compiling test for sealed class resolution with unresolvable types and additional test cases for better coverage. Co-Authored-By: Claude Opus 4.6 --- .../checks/FinalizerAttackCheckSample.java | 36 ++++++++ .../checks/FinalizerAttackCheckSample.java | 90 +++++++++++++++++++ .../java/checks/FinalizerAttackCheck.java | 22 ++--- .../java/checks/FinalizerAttackCheckTest.java | 9 ++ 4 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/FinalizerAttackCheckSample.java diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/FinalizerAttackCheckSample.java new file mode 100644 index 00000000000..e0adcfa839e --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/FinalizerAttackCheckSample.java @@ -0,0 +1,36 @@ +package checks; + +class FinalizerAttackCheckSample { + + // --- Noncompliant: sealed class permitting an unknown type (unresolvable) --- + // When the permitted type cannot be resolved, the class is conservatively treated as safely sealed. + // However, this sealed class also permits a non-sealed type that IS resolvable. + + static sealed class SealedWithUnknown permits UnknownType, KnownNonSealed { // Secondary {{Non-final class}} + public SealedWithUnknown(String s) throws Exception { // Noncompliant + if (s == null) throw new Exception(); + } + } + + static non-sealed class KnownNonSealed extends SealedWithUnknown { // Secondary {{Non-final class}} + KnownNonSealed(String s) throws Exception { // Noncompliant + super(s); + } + } + + // --- Compliant: sealed class permitting only unknown types (conservatively safe) --- + + static sealed class SealedWithOnlyUnknown permits AnotherUnknownType { + public SealedWithOnlyUnknown(String s) throws Exception { + if (s == null) throw new Exception(); + } + } + + // --- Noncompliant: non-final class with throwing constructor (basic case) --- + + static class BasicThrowing { // Secondary {{Non-final class}} + public BasicThrowing() throws Exception { // Noncompliant + throw new Exception(); + } + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java index ae6fcb63d5e..e8f768e0036 100644 --- a/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -611,4 +611,94 @@ public FieldWithoutInitializer() throws Exception { // Noncompliant throw new Exception(); } } + + // --- Compliant: sealed class with all sealed children (no non-sealed in hierarchy) --- + + static sealed class SealedAllSealed permits SealedChildA { + public SealedAllSealed(String s) throws Exception { + if (s == null) throw new Exception(); + } + } + + static final class SealedChildA extends SealedAllSealed { + SealedChildA(String s) throws Exception { + super(s); + } + } + + // --- Compliant: field initializer calls method (no direct throw in expression) --- + + static class FieldInitMethodCallOnly { + private final Object value = initOrThrow(); + + private static Object initOrThrow() { + throw new UnsupportedOperationException(); + } + } + + // --- Compliant: abstract class with private constructor and throwing initializer --- + + static abstract class AbstractPrivateCtorThrowingInit { + { + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init"); + } + } + + private AbstractPrivateCtorThrowingInit() { + } + } + + // --- Noncompliant: class with multiple constructors, one private one public, throwing initializer --- + + static class MixedCtorsThrowingInit { // Secondary {{Non-final class}} + { + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init"); + } + } + + private MixedCtorsThrowingInit(int x) { + } + + public MixedCtorsThrowingInit(String s) { // Noncompliant + } + } + + // --- Compliant: class with final finalize() and throwing field initializer calls method --- + + static class FinalFinalizerFieldInit { + private final Object value = computeValue(); + + private static Object computeValue() { + throw new UnsupportedOperationException(); + } + + @Override + protected final void finalize() { + } + } + + // --- Compliant: class with only private constructor, throwing initializer, no default ctor --- + + static class PrivateCtorOnlyThrowInit { + { + if (System.currentTimeMillis() == 0) { + throw new RuntimeException("init"); + } + } + + private PrivateCtorOnlyThrowInit(String s) { + } + } + + // --- Noncompliant: nested inner class with throwing constructor --- + + static class OuterClass { + static class InnerVulnerable { // Secondary {{Non-final class}} + public InnerVulnerable(String s) throws Exception { // Noncompliant + if (s == null) throw new Exception(); + } + } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index 7129644336c..ed031430f2d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -176,18 +176,18 @@ private static ClassTree findClassInTree(Tree tree, String name) { if (classTree.simpleName() != null && name.equals(classTree.simpleName().name())) { return classTree; } - for (Tree member : classTree.members()) { - ClassTree found = findClassInTree(member, name); - if (found != null) { - return found; - } - } + return findClassInChildren(classTree.members(), name); } else if (tree.is(Kind.COMPILATION_UNIT)) { - for (Tree child : ((CompilationUnitTree) tree).types()) { - ClassTree found = findClassInTree(child, name); - if (found != null) { - return found; - } + return findClassInChildren(((CompilationUnitTree) tree).types(), name); + } + return null; + } + + private static ClassTree findClassInChildren(List children, String name) { + for (Tree child : children) { + ClassTree found = findClassInTree(child, name); + if (found != null) { + return found; } } return null; diff --git a/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java index 8ab83cb9003..819b069bef2 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java @@ -20,6 +20,7 @@ import org.sonar.java.checks.verifier.CheckVerifier; import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; class FinalizerAttackCheckTest { @@ -40,4 +41,12 @@ void test_without_semantic() { .verifyIssues(); } + @Test + void test_non_compiling() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/FinalizerAttackCheckSample.java")) + .withCheck(new FinalizerAttackCheck()) + .verifyIssues(); + } + } From 6b66ea13330a52a5a165c4e106ec01a95226cd92 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 15:31:00 +0200 Subject: [PATCH 18/18] SONARJAVA-6783: Extract isThrowingInitializer to fix S1871 duplicate branch Co-Authored-By: Claude Opus 4.6 --- .../org/sonar/java/checks/FinalizerAttackCheck.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java index ed031430f2d..5c73fc2d35a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -70,10 +70,7 @@ private void checkMembers(ClassTree classTree, List throwing locations, null); } + private static boolean isThrowingInitializer(Tree member) { + return (member.is(Kind.INITIALIZER) && containsThrowStatementInBlock((BlockTree) member)) + || (member.is(Kind.VARIABLE) && hasThrowingFieldInitializer((VariableTree) member)); + } + private static boolean isLocalClass(ClassTree classTree) { Tree parent = classTree.parent(); while (parent != null) {