From b8cc914adce04cd622ab792dfb2298ce98e560ab Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Fri, 21 Aug 2026 10:03:15 +0200 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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); } } }