Skip to content

Commit 2ef7ec2

Browse files
java-team-github-botError Prone Team
authored and
Error Prone Team
committed
Create class to share logic between ErrorProne checkers
PiperOrigin-RevId: 552922317
1 parent c603793 commit 2ef7ec2

File tree

4 files changed

+53
-38
lines changed

4 files changed

+53
-38
lines changed
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2023 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import com.google.errorprone.VisitorState;
20+
import com.google.errorprone.matchers.Description;
21+
import com.google.errorprone.matchers.Matcher;
22+
import com.sun.source.tree.Tree;
23+
24+
/** An abstract class that detects use of the unsafe APIs. */
25+
public abstract class AbstractBanUnsafeAPIChecker extends BugChecker {
26+
27+
protected <T extends Tree> Description matchHelper(
28+
T tree, VisitorState state, Matcher<T> matcher) {
29+
if (state.errorProneOptions().isTestOnlyTarget() || !matcher.matches(tree, state)) {
30+
return Description.NO_MATCH;
31+
}
32+
33+
Description.Builder description = buildDescription(tree);
34+
35+
return description.build();
36+
}
37+
}

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

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,17 @@
3535
import com.sun.source.tree.ExpressionTree;
3636
import com.sun.source.tree.MethodInvocationTree;
3737
import com.sun.source.tree.NewClassTree;
38-
import com.sun.source.tree.Tree;
3938

4039
/** A {@link BugChecker} that detects use of the unsafe JNDI API system. */
4140
@BugPattern(
4241
summary =
4342
"Using dangerous ClassLoader APIs may deserialize untrusted user input into bytecode,"
4443
+ " leading to remote code execution vulnerabilities",
4544
severity = SeverityLevel.ERROR)
46-
public final class BanClassLoader extends BugChecker
45+
public final class BanClassLoader extends AbstractBanUnsafeAPIChecker
4746
implements MethodInvocationTreeMatcher, NewClassTreeMatcher, ClassTreeMatcher {
4847

49-
private static final Matcher<ExpressionTree> METHOD_MATCHER =
48+
private static final Matcher<MethodInvocationTree> METHOD_MATCHER =
5049
anyOf(
5150
anyMethod().onDescendantOf("java.lang.ClassLoader").named("defineClass"),
5251
anyMethod().onDescendantOf("java.lang.invoke.MethodHandles.Lookup").named("defineClass"),
@@ -63,28 +62,18 @@ public final class BanClassLoader extends BugChecker
6362
private static final Matcher<ClassTree> EXTEND_CLASS_MATCHER =
6463
isExtensionOf("java.net.URLClassLoader");
6564

66-
private <T extends Tree> Description matchWith(T tree, VisitorState state, Matcher<T> matcher) {
67-
if (state.errorProneOptions().isTestOnlyTarget() || !matcher.matches(tree, state)) {
68-
return Description.NO_MATCH;
69-
}
70-
71-
Description.Builder description = buildDescription(tree);
72-
73-
return description.build();
74-
}
75-
7665
@Override
7766
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
78-
return matchWith(tree, state, METHOD_MATCHER);
67+
return matchHelper(tree, state, METHOD_MATCHER);
7968
}
8069

8170
@Override
8271
public Description matchNewClass(NewClassTree tree, VisitorState state) {
83-
return matchWith(tree, state, CONSTRUCTOR_MATCHER);
72+
return matchHelper(tree, state, CONSTRUCTOR_MATCHER);
8473
}
8574

8675
@Override
8776
public Description matchClass(ClassTree tree, VisitorState state) {
88-
return matchWith(tree, state, EXTEND_CLASS_MATCHER);
77+
return matchHelper(tree, state, EXTEND_CLASS_MATCHER);
8978
}
9079
}

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
2626
import com.google.errorprone.matchers.Description;
2727
import com.google.errorprone.matchers.Matcher;
28-
import com.sun.source.tree.ExpressionTree;
2928
import com.sun.source.tree.MethodInvocationTree;
3029

3130
/** A {@link BugChecker} that detects use of the unsafe JNDI API system. */
@@ -34,10 +33,11 @@
3433
"Using JNDI may deserialize user input via the `Serializable` API which is extremely"
3534
+ " dangerous",
3635
severity = SeverityLevel.ERROR)
37-
public final class BanJNDI extends BugChecker implements MethodInvocationTreeMatcher {
36+
public final class BanJNDI extends AbstractBanUnsafeAPIChecker
37+
implements MethodInvocationTreeMatcher {
3838

3939
/** Checks for direct or indirect calls to context.lookup() via the JDK */
40-
private static final Matcher<ExpressionTree> MATCHER =
40+
private static final Matcher<MethodInvocationTree> MATCHER =
4141
anyOf(
4242
anyMethod()
4343
.onDescendantOf("javax.naming.directory.DirContext")
@@ -70,12 +70,6 @@ public final class BanJNDI extends BugChecker implements MethodInvocationTreeMat
7070

7171
@Override
7272
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
73-
if (state.errorProneOptions().isTestOnlyTarget() || !MATCHER.matches(tree, state)) {
74-
return Description.NO_MATCH;
75-
}
76-
77-
Description.Builder description = buildDescription(tree);
78-
79-
return description.build();
73+
return this.matchHelper(tree, state, MATCHER);
8074
}
8175
}

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

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,16 @@
3232
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
3333
import com.google.errorprone.matchers.Description;
3434
import com.google.errorprone.matchers.Matcher;
35-
import com.sun.source.tree.ExpressionTree;
3635
import com.sun.source.tree.MethodInvocationTree;
3736

3837
/** A {@link BugChecker} that detects use of the unsafe {@link java.io.Serializable} API. */
3938
@BugPattern(
4039
summary = "Deserializing user input via the `Serializable` API is extremely dangerous",
4140
severity = SeverityLevel.ERROR)
42-
public final class BanSerializableRead extends BugChecker implements MethodInvocationTreeMatcher {
41+
public final class BanSerializableRead extends AbstractBanUnsafeAPIChecker
42+
implements MethodInvocationTreeMatcher {
4343

44-
private static final Matcher<ExpressionTree> EXEMPT =
44+
private static final Matcher<MethodInvocationTree> EXEMPT =
4545
anyOf(
4646
// This is called through ObjectInputStream; a call further up the callstack will have
4747
// been exempt.
@@ -56,7 +56,7 @@ public final class BanSerializableRead extends BugChecker implements MethodInvoc
5656
methodTree.getName().toString()))));
5757

5858
/** Checks for unsafe deserialization calls on an ObjectInputStream in an ExpressionTree. */
59-
private static final Matcher<ExpressionTree> OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER =
59+
private static final Matcher<MethodInvocationTree> OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER =
6060
allOf(
6161
anyOf(
6262
// this matches calls to the ObjectInputStream to read some objects
@@ -91,16 +91,11 @@ public final class BanSerializableRead extends BugChecker implements MethodInvoc
9191
not(EXEMPT));
9292

9393
/** Checks for unsafe uses of the Java deserialization API. */
94-
private static final Matcher<ExpressionTree> MATCHER = OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER;
94+
private static final Matcher<MethodInvocationTree> MATCHER =
95+
OBJECT_INPUT_STREAM_DESERIALIZE_MATCHER;
9596

9697
@Override
9798
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
98-
if (state.errorProneOptions().isTestOnlyTarget() || !MATCHER.matches(tree, state)) {
99-
return Description.NO_MATCH;
100-
}
101-
102-
Description.Builder description = buildDescription(tree);
103-
104-
return description.build();
99+
return this.matchHelper(tree, state, MATCHER);
105100
}
106101
}

0 commit comments

Comments
 (0)