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..965fe05a3d5 --- /dev/null +++ b/its/ruling/src/test/resources/commons-beanutils/java-S9345.json @@ -0,0 +1,44 @@ +{ +"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 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanPropertyValueEqualsPredicate.java": [ +164 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanToPropertyValueTransformer.java": [ +119 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/FluentPropertyBeanIntrospector.java": [ +97 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java": [ +85, +151, +197 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/MethodUtils.java": [ +1319 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/ResultSetDynaClass.java": [ +100, +128, +159 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/RowSetDynaClass.java": [ +101, +123, +148, +176, +206, +236 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/SuppressPropertiesBeanIntrospector.java": [ +62 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/converters/ArrayConverter.java": [ +150 +] +} 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..ecb785c6447 --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9345.json @@ -0,0 +1,38 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java": [ +37 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ +125 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java": [ +36 +], +"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 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java": [ +303 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java": [ +68 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java": [ +33 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java": [ +54 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java": [ +370 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java": [ +47 +] +} 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..f183c827e6d --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9345.json @@ -0,0 +1,123 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HostPortHttpField.java": [ +37 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ +125 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java": [ +36 +], +"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 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java": [ +303 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java": [ +68 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/EncodingHttpWriter.java": [ +33 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelListeners.java": [ +54 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java": [ +370 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java": [ +47 +], +"org.eclipse.jetty:jetty-project:jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONPojoConvertorFactory.java": [ +46 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/BlockingArrayQueue.java": [ +126 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ClassLoadingObjectInputStream.java": [ +48, +53 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/CountingCallback.java": [ +45 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java": [ +37 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java": [ +85 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/InetAddressPattern.java": [ +115, +198, +241 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java": [ +43, +53 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartWriter.java": [ +41 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java": [ +55, +68, +162 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/PathWatcher.java": [ +98 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java": [ +52 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/RolloverFileOutputStream.java": [ +79, +91, +104, +120, +140, +151 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/Uptime.java": [ +41 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/component/FileDestroyable.java": [ +41 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java": [ +279, +331 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java": [ +89 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/security/CertificateValidator.java": [ +86 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java": [ +48 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/X509.java": [ +67 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java": [ +126 +], +"org.eclipse.jetty:jetty-project:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlAppendable.java": [ +38, +43, +48, +53, +58 +], +"org.eclipse.jetty:jetty-project:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java": [ +224 +] +} 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..483ffcbef99 --- /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": [ +159 +], +"com.google.guava:guava:src/com/google/common/io/MultiReader.java": [ +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 new file mode 100644 index 00000000000..1daebf5052b --- /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": [ +38 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java": [ +114 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/platform/web/MasterServletFilter.java": [ +48 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/plugins/UpdateCenterClient.java": [ +75 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/user/SecurityRealmFactory.java": [ +43 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/util/ObjectInputStreamIterator.java": [ +35 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/util/cache/DiskCache.java": [ +42 +] +} 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 new file mode 100644 index 00000000000..e8f768e0036 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/FinalizerAttackCheckSample.java @@ -0,0 +1,704 @@ +package checks; + +class FinalizerAttackCheckSample { + + // --- Noncompliant: non-final class with throwing constructor --- + + static class SecurityService { // Secondary {{Non-final class}} + private final String token; + + 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"); + } + this.token = token; + } + } + + 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 { // Secondary {{Non-final class}} + ResourceLoader(String path) { // Noncompliant + if (path == null) { + throw new NullPointerException(); + } + } + } + + static class MultiConstructorService { // Secondary {{Non-final class}} + MultiConstructorService(int id) throws Exception { // Noncompliant + if (id < 0) { + throw new Exception("Negative id"); + } + } + + MultiConstructorService(String name) { + } + } + + 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 { // Secondary {{Non-final class}} + public ThrowsClauseOnly() throws Exception { // Noncompliant + } + } + + // --- Compliant: final class --- + + static final class SecureService { + public SecureService(String token) throws IllegalArgumentException { + if (token == null) { + throw new IllegalArgumentException("Invalid token"); + } + } + } + + // --- Compliant: all constructors private (factory pattern) --- + + static 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 --- + + static class SafeService { + public SafeService(String data) { + // no throw + } + } + + static class NoConstructor { + void doSomething() { + } + } + + // --- Noncompliant: abstract class with throwing constructor (attacker can subclass) --- + + static abstract class AbstractService { // Secondary {{Non-final class}} + public AbstractService(String data) throws Exception { // Noncompliant + if (data == null) { + throw new Exception("Null"); + } + } + } + + // --- Compliant: private throwing constructor, public non-throwing constructor --- + + static 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: sealed class whose permitted subclasses are all final/sealed --- + + static sealed class SealedServiceAllFinal permits AllowedSubclass { + public SealedServiceAllFinal(String data) throws Exception { + if (data == null) { + throw new Exception("Null"); + } + } + } + + 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 { + void execute(); + } + + // --- Noncompliant: throw in constructor body without throws clause --- + + static class ConfigLoader { // Secondary {{Non-final class}} + public ConfigLoader(String config) { // Noncompliant + if (config == null) { + throw new IllegalStateException("Missing config"); + } + } + } + + // --- Compliant: throw in a method, not in constructor --- + + static class Processor { + public Processor() { + } + + public void process() { + throw new UnsupportedOperationException(); + } + } + + // --- Noncompliant: nested throw in try block within constructor --- + + static class DatabaseConnection { // Secondary {{Non-final class}} + public DatabaseConnection(String url) { // Noncompliant + try { + if (url == null) { + throw new RuntimeException("Null URL"); + } + } catch (Exception e) { + throw new RuntimeException("Connection failed", e); + } + } + } + + // --- Compliant: all throwing constructors are private --- + + static class PrivateOnlyThrowers { + private PrivateOnlyThrowers(String s) throws Exception { + throw new Exception(); + } + + 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"); + } + } + } + + // --- 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; + } + + // --- 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"); + } + } + } + + // --- 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 + } + } + + // --- 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(); + } + } + + // --- 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 new file mode 100644 index 00000000000..5c73fc2d35a --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/FinalizerAttackCheck.java @@ -0,0 +1,260 @@ +/* + * 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.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.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 { + + @Override + public List nodesToVisit() { + return Collections.singletonList(Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + ClassTree classTree = (ClassTree) tree; + if (classTree.simpleName() == null || + ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.FINAL) || + isLocalClass(classTree) || + isSafelySealedClass(classTree) || + hasFinalFinalizer(classTree)) { + return; + } + 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; + boolean hasThrowingInitializers = false; + List throwingInitializers = new ArrayList<>(); + + for (Tree member : classTree.members()) { + if (member.is(Kind.CONSTRUCTOR)) { + hasExplicitConstructor = true; + } else if (isThrowingInitializer(member)) { + throwingInitializers.add(member); + hasThrowingInitializers = true; + } + } + + if (hasExplicitConstructor) { + reportVulnerableConstructors(classTree, hasThrowingInitializers, secondaryLocations); + } else if (hasThrowingInitializers) { + reportThrowingInitializers(classTree, throwingInitializers); + } + } + + 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); + } + } + } + } + + 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 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) { + 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()) { + 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(); + } + 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; + } + return findClassInChildren(classTree.members(), name); + } else if (tree.is(Kind.COMPILATION_UNIT)) { + 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; + } + + 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, boolean hasThrowingInitializers) { + if (ModifiersUtils.hasModifier(constructor.modifiers(), Modifier.PRIVATE)) { + return false; + } + return hasThrowingInitializers || !constructor.throwsClauses().isEmpty() || containsThrowStatement(constructor); + } + + 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; + + @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..819b069bef2 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/FinalizerAttackCheckTest.java @@ -0,0 +1,52 @@ +/* + * 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; +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; + +class FinalizerAttackCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/FinalizerAttackCheckSample.java")) + .withCheck(new FinalizerAttackCheck()) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/FinalizerAttackCheckSample.java")) + .withCheck(new FinalizerAttackCheck()) + .withoutSemantic() + .verifyIssues(); + } + + @Test + void test_non_compiling() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("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..3ee4fee68ae --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.html @@ -0,0 +1,67 @@ +

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..5712e011867 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9345.json @@ -0,0 +1,29 @@ +{ + "title": "Classes with throwing constructors should be protected against Finalizer attacks", + "type": "VULNERABILITY", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "cert" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-9345", + "sqKey": "S9345", + "scope": "Main", + "defaultQualityProfiles": [ + "Sonar way" + ], + "quickfix": "unknown", + "code": { + "impacts": { + "SECURITY": "HIGH" + }, + "attribute": "COMPLETE" + }, + "securityStandards": { + "CERT": ["OBJ11-J."] + } +} 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