SONARJAVA-4758 Add exception to S1113 to cover the Finalizer Attack (#…

…4692)

java-checks-test-sources/default/src/main/files/non-compiling/checks/ObjectFinalizeOverriddenCheckSample.java

@@ -0,0 +1,63 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+class ObjectFinalizeOverriddenCheckSample {
+  class Foo {
+
+    @Override
+    protected void finalize() throws Throwable { // Noncompliant [[sc=20;ec=28]] {{Do not override the Object.finalize() method.}}
+    }
+
+    public void foo() { // Compliant
+    }
+
+    @Override
+    protected boolean finalize() { // Compliant
+    }
+
+  }
+
+  class CompliantFoo {
+
+    @Override
+    protected final void finalize() throws Throwable { // Compliant
+    }
+
+    public void bar() { // Compliant
+    }
+
+    @Override
+    protected boolean finalize() { // Compliant
+      doSomething();
+    }
+  }
+
+  class NoncompliantFoo2 {
+    @Override
+    protected final void finalize() throws Throwable { // Noncompliant
+      doSomething();
+    }
+
+  }
+
+  private void doSomething() {
+    // does something
+  }
+
+}

java-checks-test-sources/default/src/main/java/checks/ObjectFinalizeOverriddenCheckSample.java

@@ -0,0 +1,63 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+package checks;
+
+class ObjectFinalizeOverriddenCheckSample {
+  class Foo {
+
+    @Override
+    protected void finalize() throws Throwable { // Noncompliant [[sc=20;ec=28]] {{Do not override the Object.finalize() method.}}
+    }
+
+    public void foo() { // Compliant
+    }
+  }
+
+  class CompliantFoo {
+
+    @Override
+    protected final void finalize() throws Throwable { // Compliant
+    }
+
+    public void bar() { // Compliant
+    }
+  }
+
+  class NoncompliantFoo2 {
+    @Override
+    protected final void finalize() throws Throwable { // Noncompliant
+      doSomething();
+    }
+
+  }
+
+  class NoncompliantFoo3 {
+    @Override
+    protected void finalize() { // Noncompliant
+      doSomething();
+    }
+  }
+
+  private void doSomething() {
+    // does something
+  }
+
+}

java-checks/src/main/java/org/sonar/java/checks/ObjectFinalizeOverridenCheck.java → java-checks/src/main/java/org/sonar/java/checks/ObjectFinalizeOverriddenCheck.java

@@ -19,19 +19,27 @@
  */
 package org.sonar.java.checks;
 
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
 import org.sonar.check.Rule;
+import org.sonar.java.model.ModifiersUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.semantic.MethodMatchers;
 import org.sonar.plugins.java.api.tree.MethodTree;
-import org.sonar.plugins.java.api.tree.PrimitiveTypeTree;
+import org.sonar.plugins.java.api.tree.Modifier;
 import org.sonar.plugins.java.api.tree.Tree;
-
-import java.util.Collections;
-import java.util.List;
 import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey;
 
 @DeprecatedRuleKey(ruleKey = "ObjectFinalizeOverridenCheck", repositoryKey = "squid")
 @Rule(key = "S1113")
-public class ObjectFinalizeOverridenCheck extends IssuableSubscriptionVisitor {
+public class ObjectFinalizeOverriddenCheck extends IssuableSubscriptionVisitor {
+
+  private static final MethodMatchers FINALIZE_MATCHER = MethodMatchers.create()
+    .ofSubTypes("java.lang.Object")
+    .names("finalize")
+    .addWithoutParametersMatcher()
+    .build();
 
   @Override
   public List<Tree.Kind> nodesToVisit() {
@@ -41,19 +49,13 @@ public List<Tree.Kind> nodesToVisit() {
   @Override
   public void visitNode(Tree tree) {
     MethodTree methodTree = (MethodTree) tree;
-    if (isFinalize(methodTree)) {
+    if (FINALIZE_MATCHER.matches(methodTree) && isNotFinalOrHasNonEmptyBody(methodTree)) {
       reportIssue(methodTree.simpleName(), "Do not override the Object.finalize() method.");
     }
   }
 
-  private static boolean isFinalize(MethodTree methodTree) {
-    if ("finalize".equals(methodTree.simpleName().name())) {
-      Tree returnType = methodTree.returnType();
-      if (returnType != null && returnType.is(Tree.Kind.PRIMITIVE_TYPE)) {
-        return "void".equals(((PrimitiveTypeTree) returnType).keyword().text());
-      }
-    }
-    return false;
+  private static boolean isNotFinalOrHasNonEmptyBody(MethodTree methodTree) {
+    return !ModifiersUtils.hasModifier(methodTree.modifiers(), Modifier.FINAL)
+      || !Objects.requireNonNull(methodTree.block()).body().isEmpty();
   }
-
 }

java-checks/src/test/java/org/sonar/java/checks/ObjectFinalizeOverridenCheckTest.java → java-checks/src/test/java/org/sonar/java/checks/ObjectFinalizeOverriddenCheckTest.java

@@ -21,13 +21,21 @@
 
 import org.junit.jupiter.api.Test;
 import org.sonar.java.checks.verifier.CheckVerifier;
+import org.sonar.java.checks.verifier.TestUtils;
 
-class ObjectFinalizeOverridenCheckTest {
+class ObjectFinalizeOverriddenCheckTest {
   @Test
-  void test() {
+  void test_non_compiling() {
     CheckVerifier.newVerifier()
-      .onFile("src/test/files/checks/ObjectFinalizeOverridenCheck.java")
-      .withCheck(new ObjectFinalizeOverridenCheck())
+      .onFile(TestUtils.nonCompilingTestSourcesPath("checks/ObjectFinalizeOverriddenCheckSample.java"))
+      .withCheck(new ObjectFinalizeOverriddenCheck())
+      .verifyIssues();
+  }
+  @Test
+  void test_compiling() {
+    CheckVerifier.newVerifier()
+      .onFile(TestUtils.mainCodeSourcesPath("checks/ObjectFinalizeOverriddenCheckSample.java"))
+      .withCheck(new ObjectFinalizeOverriddenCheck())
       .verifyIssues();
   }
 }

sonar-java-plugin/src/main/java/org/sonar/plugins/java/CheckList.java

@@ -270,8 +270,8 @@
 import org.sonar.java.checks.ObjectCreatedOnlyToCallGetClassCheck;
 import org.sonar.java.checks.ObjectFinalizeCheck;
 import org.sonar.java.checks.ObjectFinalizeOverloadedCheck;
+import org.sonar.java.checks.ObjectFinalizeOverriddenCheck;
 import org.sonar.java.checks.ObjectFinalizeOverridenCallsSuperFinalizeCheck;
-import org.sonar.java.checks.ObjectFinalizeOverridenCheck;
 import org.sonar.java.checks.ObjectFinalizeOverridenNotPublicCheck;
 import org.sonar.java.checks.OctalValuesCheck;
 import org.sonar.java.checks.OmitPermittedTypesCheck;
@@ -977,7 +977,7 @@ public final class CheckList {
     ObjectFinalizeCheck.class,
     ObjectFinalizeOverloadedCheck.class,
     ObjectFinalizeOverridenCallsSuperFinalizeCheck.class,
-    ObjectFinalizeOverridenCheck.class,
+    ObjectFinalizeOverriddenCheck.class,
     ObjectFinalizeOverridenNotPublicCheck.class,
     OmitPermittedTypesCheck.class,
     OneClassInterfacePerFileCheck.class,

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S1113.html

@@ -17,12 +17,15 @@ <h3>Noncompliant code example</h3>
 public class MyClass {
 
   @Override
-  protected void finalize() {
-    releaseSomeResources();    // Noncompliant
+  protected void finalize() { // Noncompliant
+    releaseSomeResources();
   }
 
 }
 </pre>
+<h3>Exceptions</h3>
+<p>It is allowed to override the <code>finalize()</code> method as <code>final</code> method with an empty body, to prevent the <em>finalizer
+attack</em> as described in <em>MET12-J-EX1</em>.</p>
 <h2>Resources</h2>
 <ul>
   <li> <a href="https://docs.oracle.com/javase/specs/jls/se17/html/jls-12.html#jls-12.6">docs.oracle.com</a> - Finalization of Class Instances </li>