Create class to share logic between ErrorProne checkers

PiperOrigin-RevId: 552922317

core/src/main/java/com/google/errorprone/bugpatterns/AbstractBanUnsafeAPIChecker.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2023 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.Tree;
+
+/** An abstract class that detects use of the unsafe APIs. */
+public abstract class AbstractBanUnsafeAPIChecker extends BugChecker {
+
+  protected <T extends Tree> Description matchHelper(
+      T tree, VisitorState state, Matcher<T> matcher) {
+    if (state.errorProneOptions().isTestOnlyTarget() || !matcher.matches(tree, state)) {
+      return Description.NO_MATCH;
+    }
+
+    Description.Builder description = buildDescription(tree);
+
+    return description.build();
+  }
+}

core/src/main/java/com/google/errorprone/bugpatterns/BanClassLoader.java

@@ -35,18 +35,17 @@
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.NewClassTree;
-import com.sun.source.tree.Tree;
 
 /** A {@link BugChecker} that detects use of the unsafe JNDI API system. */
 @BugPattern(
     summary =
         "Using dangerous ClassLoader APIs may deserialize untrusted user input into bytecode,"
             + " leading to remote code execution vulnerabilities",
     severity = SeverityLevel.ERROR)
-public final class BanClassLoader extends BugChecker
+public final class BanClassLoader extends AbstractBanUnsafeAPIChecker
     implements MethodInvocationTreeMatcher, NewClassTreeMatcher, ClassTreeMatcher {
 
-  private static final Matcher<ExpressionTree> METHOD_MATCHER =
+  private static final Matcher<MethodInvocationTree> METHOD_MATCHER =
       anyOf(
           anyMethod().onDescendantOf("java.lang.ClassLoader").named("defineClass"),
           anyMethod().onDescendantOf("java.lang.invoke.MethodHandles.Lookup").named("defineClass"),
@@ -63,28 +62,18 @@ public final class BanClassLoader extends BugChecker
   private static final Matcher<ClassTree> EXTEND_CLASS_MATCHER =
       isExtensionOf("java.net.URLClassLoader");
 
-  private <T extends Tree> Description matchWith(T tree, VisitorState state, Matcher<T> matcher) {
-    if (state.errorProneOptions().isTestOnlyTarget() || !matcher.matches(tree, state)) {
-      return Description.NO_MATCH;
-    }
-
-    Description.Builder description = buildDescription(tree);
-
-    return description.build();
-  }
-
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
-    return matchWith(tree, state, METHOD_MATCHER);
+    return matchHelper(tree, state, METHOD_MATCHER);
   }
 
   @Override
   public Description matchNewClass(NewClassTree tree, VisitorState state) {
-    return matchWith(tree, state, CONSTRUCTOR_MATCHER);
+    return matchHelper(tree, state, CONSTRUCTOR_MATCHER);
   }
 
   @Override
   public Description matchClass(ClassTree tree, VisitorState state) {
-    return matchWith(tree, state, EXTEND_CLASS_MATCHER);
+    return matchHelper(tree, state, EXTEND_CLASS_MATCHER);
   }
 }

core/src/main/java/com/google/errorprone/bugpatterns/BanJNDI.java

@@ -25,7 +25,6 @@
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
-import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
 
 /** A {@link BugChecker} that detects use of the unsafe JNDI API system. */
@@ -34,10 +33,11 @@
         "Using JNDI may deserialize user input via the `Serializable` API which is extremely"
             + " dangerous",
     severity = SeverityLevel.ERROR)
-public final class BanJNDI extends BugChecker implements MethodInvocationTreeMatcher {
+public final class BanJNDI extends AbstractBanUnsafeAPIChecker
+    implements MethodInvocationTreeMatcher {
 
   /** Checks for direct or indirect calls to context.lookup() via the JDK */
-  private static final Matcher<ExpressionTree> MATCHER =
+  private static final Matcher<MethodInvocationTree> MATCHER =
       anyOf(
           anyMethod()
               .onDescendantOf("javax.naming.directory.DirContext")
@@ -70,12 +70,6 @@ public final class BanJNDI extends BugChecker implements MethodInvocationTreeMat
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
-    if (state.errorProneOptions().isTestOnlyTarget() || !MATCHER.matches(tree, state)) {
-      return Description.NO_MATCH;
-    }
-
-    Description.Builder description = buildDescription(tree);
-
-    return description.build();
+    return this.matchHelper(tree, state, MATCHER);
   }
 }

core/src/main/java/com/google/errorprone/bugpatterns/BanSerializableRead.java

@@ -32,16 +32,16 @@
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
-import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
 
 /** A {@link BugChecker} that detects use of the unsafe {@link java.io.Serializable} API. */
 @BugPattern(
     summary = "Deserializing user input via the `Serializable` API is extremely dangerous",
     severity = SeverityLevel.ERROR)
-public final class BanSerializableRead extends BugChecker implements MethodInvocationTreeMatcher {
+public final class BanSerializableRead extends AbstractBanUnsafeAPIChecker
+    implements MethodInvocationTreeMatcher {
 
-  private static final Matcher<ExpressionTree> EXEMPT =
+  private static final Matcher<MethodInvocationTree> EXEMPT =
       anyOf(
           //  This is called through ObjectInputStream; a call further up the callstack will have
           // been exempt.
@@ -56,7 +56,7 @@ public final class BanSerializableRead extends BugChecker implements MethodInvoc
                           methodTree.getName().toString()))));
 
   /** Checks for unsafe deserialization calls on an ObjectInputStream in an ExpressionTree. */
-  private static final Matcher<ExpressionTree> OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER =
+  private static final Matcher<MethodInvocationTree> OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER =
       allOf(
           anyOf(
               // this matches calls to the ObjectInputStream to read some objects
@@ -91,16 +91,11 @@ public final class BanSerializableRead extends BugChecker implements MethodInvoc
           not(EXEMPT));
 
   /** Checks for unsafe uses of the Java deserialization API. */
-  private static final Matcher<ExpressionTree> MATCHER = OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER;
+  private static final Matcher<MethodInvocationTree> MATCHER =
+      OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER;
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
-    if (state.errorProneOptions().isTestOnlyTarget() || !MATCHER.matches(tree, state)) {
-      return Description.NO_MATCH;
-    }
-
-    Description.Builder description = buildDescription(tree);
-
-    return description.build();
+    return this.matchHelper(tree, state, MATCHER);
   }
 }