Skip to content

Commit 848ff93

Browse files
authored
Added a Java deserialization remediator (#432)
1 parent f8af718 commit 848ff93

File tree

3 files changed

+291
-0
lines changed

3 files changed

+291
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package io.codemodder.remediation.javadeserialization;
2+
3+
import static io.codemodder.javaparser.JavaParserTransformer.replace;
4+
5+
import com.github.javaparser.ast.CompilationUnit;
6+
import com.github.javaparser.ast.Node;
7+
import com.github.javaparser.ast.body.VariableDeclarator;
8+
import com.github.javaparser.ast.expr.Expression;
9+
import com.github.javaparser.ast.expr.MethodCallExpr;
10+
import com.github.javaparser.ast.expr.ObjectCreationExpr;
11+
import io.codemodder.CodemodChange;
12+
import io.codemodder.CodemodFileScanningResult;
13+
import io.codemodder.DependencyGAV;
14+
import io.codemodder.ast.ASTs;
15+
import io.codemodder.ast.LocalDeclaration;
16+
import io.codemodder.codetf.DetectorRule;
17+
import io.codemodder.codetf.FixedFinding;
18+
import io.codemodder.codetf.UnfixedFinding;
19+
import io.codemodder.remediation.FixCandidate;
20+
import io.codemodder.remediation.FixCandidateSearchResults;
21+
import io.codemodder.remediation.FixCandidateSearcher;
22+
import io.github.pixee.security.ObjectInputFilters;
23+
import java.util.ArrayList;
24+
import java.util.List;
25+
import java.util.Optional;
26+
import java.util.function.Function;
27+
28+
final class DefaultJavaDeserializationRemediator implements JavaDeserializationRemediator {
29+
30+
@Override
31+
public <T> CodemodFileScanningResult remediateAll(
32+
final CompilationUnit cu,
33+
final String path,
34+
final DetectorRule detectorRule,
35+
final List<T> issuesForFile,
36+
final Function<T, String> getKey,
37+
final Function<T, Integer> getLine,
38+
final Function<T, Integer> getColumn) {
39+
FixCandidateSearcher<T> searcher =
40+
new FixCandidateSearcher.Builder<T>()
41+
.withMethodName("readObject")
42+
.withMatcher(mce -> mce.getScope().isPresent())
43+
.withMatcher(mce -> mce.getArguments().isEmpty())
44+
.build();
45+
46+
FixCandidateSearchResults<T> results =
47+
searcher.search(cu, path, detectorRule, issuesForFile, getKey, getLine, getColumn);
48+
49+
List<CodemodChange> changes = new ArrayList<>();
50+
List<UnfixedFinding> unfixedFindings = new ArrayList<>();
51+
for (FixCandidate<T> fixCandidate : results.fixCandidates()) {
52+
List<T> issues = fixCandidate.issues();
53+
MethodCallExpr call = fixCandidate.methodCall();
54+
// get the declaration of the ObjectInputStream
55+
Expression callScope = call.getScope().get();
56+
if (!callScope.isNameExpr()) {
57+
// can't fix these
58+
issues.stream()
59+
.map(
60+
i ->
61+
new UnfixedFinding(
62+
getKey.apply(i), detectorRule, path, getLine.apply(i), "Unexpected shape"))
63+
.forEach(unfixedFindings::add);
64+
continue;
65+
}
66+
67+
Optional<LocalDeclaration> declaration =
68+
ASTs.findEarliestLocalDeclarationOf(callScope.asNameExpr().getName());
69+
if (declaration.isEmpty()) {
70+
issues.stream()
71+
.map(
72+
i ->
73+
new UnfixedFinding(
74+
getKey.apply(i),
75+
detectorRule,
76+
path,
77+
getLine.apply(i),
78+
"No declaration found"))
79+
.forEach(unfixedFindings::add);
80+
continue;
81+
}
82+
83+
LocalDeclaration localDeclaration = declaration.get();
84+
Node varDeclarationAndExpr = localDeclaration.getDeclaration();
85+
if (varDeclarationAndExpr instanceof VariableDeclarator varDec) {
86+
Optional<Expression> initializer = varDec.getInitializer();
87+
if (initializer.isEmpty()) {
88+
issues.stream()
89+
.map(
90+
i ->
91+
new UnfixedFinding(
92+
getKey.apply(i),
93+
detectorRule,
94+
path,
95+
getLine.apply(i),
96+
"No initializer found"))
97+
.forEach(unfixedFindings::add);
98+
continue;
99+
}
100+
101+
Expression expression = initializer.get();
102+
if (expression instanceof ObjectCreationExpr objCreation) {
103+
fixObjectInputStreamCreation(objCreation);
104+
CodemodChange change =
105+
CodemodChange.from(
106+
getLine.apply(issues.get(0)),
107+
List.of(DependencyGAV.JAVA_SECURITY_TOOLKIT),
108+
issues.stream()
109+
.map(i -> new FixedFinding(getKey.apply(i), detectorRule))
110+
.toList());
111+
changes.add(change);
112+
}
113+
} else {
114+
issues.stream()
115+
.map(
116+
i ->
117+
new UnfixedFinding(
118+
getKey.apply(i),
119+
detectorRule,
120+
path,
121+
getLine.apply(i),
122+
"Unexpected declaration type"))
123+
.forEach(unfixedFindings::add);
124+
}
125+
}
126+
return CodemodFileScanningResult.from(changes, unfixedFindings);
127+
}
128+
129+
private void fixObjectInputStreamCreation(final ObjectCreationExpr objCreation) {
130+
replace(objCreation)
131+
.withStaticMethod(ObjectInputFilters.class.getName(), "createSafeObjectInputStream")
132+
.withStaticImport()
133+
.withSameArguments();
134+
}
135+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package io.codemodder.remediation.javadeserialization;
2+
3+
import com.github.javaparser.ast.CompilationUnit;
4+
import io.codemodder.CodemodFileScanningResult;
5+
import io.codemodder.codetf.DetectorRule;
6+
import java.util.List;
7+
import java.util.function.Function;
8+
9+
/** Remediates Java deserialization vulnerabilities. */
10+
public interface JavaDeserializationRemediator {
11+
12+
/** Remediate all Java deserialization vulnerabilities in the given compilation unit. */
13+
<T> CodemodFileScanningResult remediateAll(
14+
CompilationUnit cu,
15+
String path,
16+
DetectorRule detectorRule,
17+
List<T> issuesForFile,
18+
Function<T, String> getKey,
19+
Function<T, Integer> getLine,
20+
Function<T, Integer> getColumn);
21+
22+
/** The default header injection remediation strategy. */
23+
JavaDeserializationRemediator DEFAULT = new DefaultJavaDeserializationRemediator();
24+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
package io.codemodder.remediation.javadeserialization;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import com.github.javaparser.StaticJavaParser;
6+
import com.github.javaparser.ast.CompilationUnit;
7+
import com.github.javaparser.printer.lexicalpreservation.LexicalPreservingPrinter;
8+
import io.codemodder.CodemodChange;
9+
import io.codemodder.CodemodFileScanningResult;
10+
import io.codemodder.DependencyGAV;
11+
import io.codemodder.codetf.DetectorRule;
12+
import io.codemodder.codetf.FixedFinding;
13+
import io.codemodder.codetf.UnfixedFinding;
14+
import java.util.List;
15+
import java.util.stream.Stream;
16+
import org.junit.jupiter.api.BeforeEach;
17+
import org.junit.jupiter.api.Test;
18+
import org.junit.jupiter.params.ParameterizedTest;
19+
import org.junit.jupiter.params.provider.Arguments;
20+
import org.junit.jupiter.params.provider.MethodSource;
21+
22+
final class DefaultJavaDeserializationRemediatorTest {
23+
24+
private DefaultJavaDeserializationRemediator remediator;
25+
private DetectorRule rule;
26+
27+
@BeforeEach
28+
void setup() {
29+
remediator = new DefaultJavaDeserializationRemediator();
30+
rule = new DetectorRule("untrusted-deserialization", "Untrusted Deserialization", null);
31+
}
32+
33+
private static Stream<Arguments> unfixableSamples() {
34+
return Stream.of(
35+
Arguments.of(
36+
"""
37+
import java.io.ObjectInputStream;
38+
class Foo {
39+
void bar(ObjectInputStream ois) {
40+
Acme acme = ois.readObject();
41+
}
42+
}
43+
""",
44+
4,
45+
"Unexpected declaration type"),
46+
Arguments.of(
47+
"""
48+
import java.io.ObjectInputStream;
49+
class Foo {
50+
void bar() {
51+
Acme acme = getOis().readObject();
52+
}
53+
}
54+
""",
55+
4,
56+
"Unexpected shape"));
57+
}
58+
59+
@ParameterizedTest
60+
@MethodSource("unfixableSamples")
61+
void it_doesnt_handle_unfixables(final String badCode, final int line, final String reason) {
62+
CompilationUnit cu = StaticJavaParser.parse(badCode);
63+
LexicalPreservingPrinter.setup(cu);
64+
65+
CodemodFileScanningResult result =
66+
remediator.remediateAll(
67+
cu, "path", rule, List.of(new Object()), o -> "id", o -> line, o -> null);
68+
assertThat(result.unfixedFindings()).hasSize(1);
69+
assertThat(result.changes()).isEmpty();
70+
UnfixedFinding unfixedFinding = result.unfixedFindings().get(0);
71+
assertThat(unfixedFinding.getReason()).isEqualTo(reason);
72+
assertThat(unfixedFinding.getRule()).isEqualTo(rule);
73+
assertThat(unfixedFinding.getLine()).isEqualTo(line);
74+
assertThat(unfixedFinding.getPath()).isEqualTo("path");
75+
}
76+
77+
@Test
78+
void it_fixes_java_deserialization() {
79+
80+
String fixableCode =
81+
"""
82+
package com.acme;
83+
import java.io.ObjectInputStream;
84+
import java.io.InputStream;
85+
86+
class Foo {
87+
Acme readAcme(InputStream is) {
88+
ObjectInputStream ois = new ObjectInputStream(is);
89+
// read the obj
90+
Acme acme = (Acme) ois.readObject();
91+
return acme;
92+
}
93+
}
94+
""";
95+
96+
CompilationUnit cu = StaticJavaParser.parse(fixableCode);
97+
LexicalPreservingPrinter.setup(cu);
98+
99+
CodemodFileScanningResult result =
100+
remediator.remediateAll(
101+
cu, "path", rule, List.of(new Object()), o -> "id", o -> 9, o -> null);
102+
assertThat(result.unfixedFindings()).isEmpty();
103+
assertThat(result.changes()).hasSize(1);
104+
CodemodChange change = result.changes().get(0);
105+
assertThat(change.lineNumber()).isEqualTo(9);
106+
List<FixedFinding> fixedFindings = change.getFixedFindings();
107+
assertThat(fixedFindings).hasSize(1);
108+
assertThat(change.getDependenciesNeeded()).containsExactly(DependencyGAV.JAVA_SECURITY_TOOLKIT);
109+
110+
assertThat(fixedFindings.get(0).getId()).isEqualTo("id");
111+
assertThat(fixedFindings.get(0).getRule()).isEqualTo(rule);
112+
113+
String afterCode = LexicalPreservingPrinter.print(cu);
114+
assertThat(afterCode)
115+
.isEqualToIgnoringWhitespace(
116+
"""
117+
package com.acme;
118+
import static io.github.pixee.security.ObjectInputFilters.createSafeObjectInputStream;
119+
import java.io.ObjectInputStream;
120+
import java.io.InputStream;
121+
122+
class Foo {
123+
Acme readAcme(InputStream is) {
124+
ObjectInputStream ois = createSafeObjectInputStream(is);
125+
// read the obj
126+
Acme acme = (Acme) ois.readObject();
127+
return acme;
128+
}
129+
}
130+
""");
131+
}
132+
}

0 commit comments

Comments
 (0)