diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractBanUnsafeAPIChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractBanUnsafeAPIChecker.java new file mode 100644 index 000000000000..fc5bc7c6e5c5 --- /dev/null +++ b/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 Description matchHelper( + T tree, VisitorState state, Matcher matcher) { + if (state.errorProneOptions().isTestOnlyTarget() || !matcher.matches(tree, state)) { + return Description.NO_MATCH; + } + + Description.Builder description = buildDescription(tree); + + return description.build(); + } +} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/BanClassLoader.java b/core/src/main/java/com/google/errorprone/bugpatterns/BanClassLoader.java index 2c9e02615264..f7528a7dd9f5 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/BanClassLoader.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/BanClassLoader.java @@ -35,7 +35,6 @@ 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( @@ -43,10 +42,10 @@ "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 METHOD_MATCHER = + private static final Matcher 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 EXTEND_CLASS_MATCHER = isExtensionOf("java.net.URLClassLoader"); - private Description matchWith(T tree, VisitorState state, Matcher 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); } } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/BanJNDI.java b/core/src/main/java/com/google/errorprone/bugpatterns/BanJNDI.java index 3b327a4a7e1e..6d8152a91eaa 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/BanJNDI.java +++ b/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 MATCHER = + private static final Matcher 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); } } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/BanSerializableRead.java b/core/src/main/java/com/google/errorprone/bugpatterns/BanSerializableRead.java index 8e5d3ab19012..c050c9f9606a 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/BanSerializableRead.java +++ b/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 EXEMPT = + private static final Matcher 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 OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER = + private static final Matcher 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 MATCHER = OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER; + private static final Matcher 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); } }