Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the DangerousJsonTypeInfoUsage check #1557

Merged
merged 2 commits into from
Nov 23, 2020
Merged

Improve the DangerousJsonTypeInfoUsage check #1557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5875a80
Improve the DangerousJsonTypeInfoUsage check
carterkozak Nov 22, 2020
61212d3
Add generated changelog entries
carterkozak Nov 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 37 additions & 14 deletions ...rror-prone/src/main/java/com/palantir/baseline/errorprone/DangerousJsonTypeInfoUsage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,42 +23,53 @@
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.AnnotationHasArgumentWithValue;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.IsSameType;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.matchers.method.MethodMatchers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Symbol;

@AutoService(BugChecker.class)
@BugPattern(
name = "DangerousJsonTypeInfoUsage",
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
linkType = BugPattern.LinkType.CUSTOM,
severity = SeverityLevel.ERROR,
summary = "Disallow usage of Jackson's JsonTypeInfo.Id.CLASS annotation for security reasons, "
summary = "Disallow usage of Jackson's Type Information features for security reasons, "
+ "cf. https://github.com/FasterXML/jackson-databind/issues/1599")
public final class DangerousJsonTypeInfoUsage extends BugChecker implements BugChecker.AnnotationTreeMatcher {
public final class DangerousJsonTypeInfoUsage extends BugChecker
implements BugChecker.AnnotationTreeMatcher, BugChecker.MethodInvocationTreeMatcher {

private static final long serialVersionUID = 1L;

private static final Matcher<AnnotationTree> matcher = new AnnotationHasArgumentWithValue(
private static final Matcher<AnnotationTree> annotationMatcher = new AnnotationHasArgumentWithValue(
"use",
Matchers.allOf(
new IsSameType<>("com.fasterxml.jackson.annotation.JsonTypeInfo$Id"),
Matchers.anyOf(
treeEqualsStringMatcher("JsonTypeInfo.Id.CLASS"),
treeEqualsStringMatcher("JsonTypeInfo.Id.MINIMAL_CLASS"),
treeEqualsStringMatcher("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.CLASS"),
treeEqualsStringMatcher(
"com.fasterxml.jackson.annotation.JsonTypeInfo.Id.MINIMAL_CLASS"))));
Matchers.isSameType("com.fasterxml.jackson.annotation.JsonTypeInfo$Id"),
Matchers.anyOf(symbolNamed("CLASS"), symbolNamed("MINIMAL_CLASS"))));

private static Matcher<ExpressionTree> treeEqualsStringMatcher(String value) {
return (expressionTree, state) -> state.getSourceForNode(expressionTree).equals(value);
private static final Matcher<ExpressionTree> objectMapperTypeInfoMatcher = MethodMatchers.instanceMethod()
.onDescendantOf("com.fasterxml.jackson.databind.ObjectMapper")
.namedAnyOf(
"enableDefaultTyping",
"enableDefaultTypingAsProperty",
"activateDefaultTyping",
"activateDefaultTypingAsProperty",
"setDefaultTyping");

private static Matcher<ExpressionTree> symbolNamed(String value) {
return (expressionTree, state) -> {
Symbol symbol = ASTHelpers.getSymbol(expressionTree);
return symbol != null && symbol.name.contentEquals(value);
};
}

@Override
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
if (!matcher.matches(tree, state)) {
if (!annotationMatcher.matches(tree, state)) {
return Description.NO_MATCH;
}

Expand All @@ -67,4 +78,16 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
+ "JsonTypeInfo.Id.CLASS or JsonTypeInfo.Id.MINIMAL_CLASS")
.build();
}

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!objectMapperTypeInfoMatcher.matches(tree, state)) {
return Description.NO_MATCH;
}
return buildDescription(tree)
.setMessage("Must not use a Jackson ObjectMapper with default typings because it may allow remote "
+ "code execution upon deserialization. Additionally, using java types in API makes usage "
+ "more difficult for consumers using other languages.")
.build();
}
}
138 changes: 126 additions & 12 deletions ...prone/src/test/java/com/palantir/baseline/errorprone/DangerousJsonTypeInfoUsageTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,10 @@
package com.palantir.baseline.errorprone;

import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public final class DangerousJsonTypeInfoUsageTests {

private CompilationTestHelper compilationHelper;

@BeforeEach
public void before() {
compilationHelper = CompilationTestHelper.newInstance(DangerousJsonTypeInfoUsage.class, getClass());
}

@Test
public void testClass() {
positive("JsonTypeInfo.Id.CLASS");
Expand All @@ -49,6 +41,54 @@ public void testMinimalClass_fullyQualified() {
positive("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.MINIMAL_CLASS");
}

@Test
public void testClass_IdQualified() {
helper().addSourceLines(
"Bean.java",
"import com.fasterxml.jackson.annotation.JsonTypeInfo;",
"import com.fasterxml.jackson.annotation.JsonTypeInfo.Id;",
"// BUG: Diagnostic contains: Must not use Jackson @JsonTypeInfo annotation",
"@JsonTypeInfo(use = Id.CLASS)",
"class Bean {}")
.doTest();
}

@Test
public void testMinimalClass_IdQualified() {
helper().addSourceLines(
"Bean.java",
"import com.fasterxml.jackson.annotation.JsonTypeInfo;",
"import com.fasterxml.jackson.annotation.JsonTypeInfo.Id;",
"// BUG: Diagnostic contains: Must not use Jackson @JsonTypeInfo annotation",
"@JsonTypeInfo(use = Id.MINIMAL_CLASS)",
"class Bean {}")
.doTest();
}

@Test
public void testClass_ClassQualified() {
helper().addSourceLines(
"Bean.java",
"import com.fasterxml.jackson.annotation.JsonTypeInfo;",
"import static com.fasterxml.jackson.annotation.JsonTypeInfo.Id.CLASS;",
"// BUG: Diagnostic contains: Must not use Jackson @JsonTypeInfo annotation",
"@JsonTypeInfo(use = CLASS)",
"class Bean {}")
.doTest();
}

@Test
public void testMinimalClass_MinimalClassQualified() {
helper().addSourceLines(
"Bean.java",
"import com.fasterxml.jackson.annotation.JsonTypeInfo;",
"import static com.fasterxml.jackson.annotation.JsonTypeInfo.Id.MINIMAL_CLASS;",
"// BUG: Diagnostic contains: Must not use Jackson @JsonTypeInfo annotation",
"@JsonTypeInfo(use = MINIMAL_CLASS)",
"class Bean {}")
.doTest();
}

@Test
public void testNone() {
negative("JsonTypeInfo.Id.NONE");
Expand Down Expand Up @@ -79,9 +119,80 @@ public void testCustom_fullyQualified() {
negative("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.CUSTOM");
}

@Test
public void testObjectMapper_enableDefaultTyping() {
helper().addSourceLines(
"Test.java",
"import com.fasterxml.jackson.databind.ObjectMapper;",
"class Test {",
"// BUG: Diagnostic contains: Must not use a Jackson ObjectMapper with default typings",
" Object om = new ObjectMapper().enableDefaultTyping();",
"}")
.doTest();
}

@Test
public void testObjectMapper_enableDefaultTypingAsProperty() {
helper().addSourceLines(
"Test.java",
"import com.fasterxml.jackson.databind.ObjectMapper;",
"class Test {",
" Object om = new ObjectMapper()",
"// BUG: Diagnostic contains: Must not use a Jackson ObjectMapper with default typings",
" .enableDefaultTypingAsProperty(ObjectMapper.DefaultTyping.JAVA_LANG_OBJECT, \"prop\");",
"}")
.doTest();
}

@Test
public void testObjectMapper_activateDefaultTyping() {
helper().addSourceLines(
"Test.java",
"import com.fasterxml.jackson.databind.ObjectMapper;",
"import com.fasterxml.jackson.databind.jsontype.DefaultBaseTypeLimitingValidator;",
"class Test {",
"// BUG: Diagnostic contains: Must not use a Jackson ObjectMapper with default typings",
" Object om = new ObjectMapper().activateDefaultTyping(new"
+ " DefaultBaseTypeLimitingValidator());",
"}")
.doTest();
}

@Test
public void testObjectMapper_activateDefaultTypingAsProperty() {
helper().addSourceLines(
"Test.java",
"import com.fasterxml.jackson.databind.ObjectMapper;",
"import com.fasterxml.jackson.databind.jsontype.DefaultBaseTypeLimitingValidator;",
"class Test {",
" Object om = new ObjectMapper()",
"// BUG: Diagnostic contains: Must not use a Jackson ObjectMapper with default typings",
" .activateDefaultTypingAsProperty(",
" new DefaultBaseTypeLimitingValidator(),",
" ObjectMapper.DefaultTyping.JAVA_LANG_OBJECT,",
" \"prop\");",
"}")
.doTest();
}

@Test
public void testObjectMapper_setDefaultTyping() {
helper().addSourceLines(
"Test.java",
"import com.fasterxml.jackson.databind.ObjectMapper;",
"import com.fasterxml.jackson.databind.jsontype.TypeResolverBuilder;",
"class Test {",
" void f(TypeResolverBuilder value) {",
" new ObjectMapper()",
"// BUG: Diagnostic contains: Must not use a Jackson ObjectMapper with default typings",
" .setDefaultTyping(value);",
" }",
"}")
.doTest();
}

private void positive(String variant) {
compilationHelper
.addSourceLines(
helper().addSourceLines(
"Bean.java",
"import com.fasterxml.jackson.annotation.JsonTypeInfo;",
"// BUG: Diagnostic contains: Must not use Jackson @JsonTypeInfo annotation",
Expand All @@ -91,12 +202,15 @@ private void positive(String variant) {
}

private void negative(String variant) {
compilationHelper
.addSourceLines(
helper().addSourceLines(
"Bean.java",
"import com.fasterxml.jackson.annotation.JsonTypeInfo;",
"@JsonTypeInfo(use = " + variant + ")",
"class Bean {}")
.doTest();
}

private CompilationTestHelper helper() {
return CompilationTestHelper.newInstance(DangerousJsonTypeInfoUsage.class, getClass());
}
}
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-1557.v2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: fix
fix:
description: Improve the DangerousJsonTypeInfoUsage check
links:
- https://github.com/palantir/gradle-baseline/pull/1557