Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 174ffae

Browse filesBrowse files
nahsranahsra
authored
Fix more XXE cases (#488)
Fixes cases of XXE where the tool points to the `XMLInputFactory#newInstance()` call.
1 parent bb3eced commit 174ffae
Copy full SHA for 174ffae

File tree

Expand file treeCollapse file tree

8 files changed

+215
-71
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+215
-71
lines changed
Open diff view settings
Collapse file

‎core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLXXECodemod.java‎

Copy file name to clipboardExpand all lines: core-codemods/src/main/java/io/codemodder/codemods/codeql/CodeQLXXECodemod.java
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import io.codemodder.providers.sarif.codeql.ProvidedCodeQLScan;
88
import io.codemodder.remediation.GenericRemediationMetadata;
99
import io.codemodder.remediation.Remediator;
10-
import io.codemodder.remediation.xxe.XXEIntermediateXMLStreamReaderRemediator;
10+
import io.codemodder.remediation.xxe.XMLStreamReaderIntermediateRemediator;
1111
import java.util.Optional;
1212
import javax.inject.Inject;
1313

@@ -24,7 +24,7 @@ public final class CodeQLXXECodemod extends CodeQLRemediationCodemod {
2424
@Inject
2525
public CodeQLXXECodemod(@ProvidedCodeQLScan(ruleId = "java/xxe") final RuleSarif sarif) {
2626
super(GenericRemediationMetadata.XXE.reporter(), sarif);
27-
this.remediator = new XXEIntermediateXMLStreamReaderRemediator<>();
27+
this.remediator = new XMLStreamReaderIntermediateRemediator<>();
2828
}
2929

3030
@Override
Collapse file

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/TransformerFactoryAtCreationFixStrategy.java‎

Copy file name to clipboardExpand all lines: framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/TransformerFactoryAtCreationFixStrategy.java
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import java.util.Optional;
2222

2323
/**
24-
* Fix strategy for XXE vulnerabilities anchored to the TransformerParser newInstance() calls. Finds
24+
* Fix strategy for XXE vulnerabilities anchored to the TransformerParser.newInstance() calls. Finds
2525
* the parser's declaration and add statements disabling external entities and features.
2626
*/
2727
final class TransformerFactoryAtCreationFixStrategy extends MatchAndFixStrategy {
@@ -70,7 +70,7 @@ public SuccessOrReason fix(final CompilationUnit cu, final Node node) {
7070
}
7171

7272
/**
73-
* Matches against TransformerFactory.newInstance() calls
73+
* Matches against XMLInputFactory.newInstance() calls.
7474
*
7575
* @param node
7676
* @return
Collapse file

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XMLInputFactoryAtNewInstanceFixStrategy.java‎

Copy file name to clipboard
+58Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package io.codemodder.remediation.xxe;
2+
3+
import com.github.javaparser.ast.CompilationUnit;
4+
import com.github.javaparser.ast.Node;
5+
import com.github.javaparser.ast.body.VariableDeclarator;
6+
import com.github.javaparser.ast.expr.Expression;
7+
import com.github.javaparser.ast.expr.MethodCallExpr;
8+
import com.github.javaparser.ast.stmt.Statement;
9+
import io.codemodder.ast.ASTs;
10+
import io.codemodder.remediation.SuccessOrReason;
11+
import java.util.List;
12+
import java.util.Optional;
13+
14+
final class XMLInputFactoryAtNewInstanceFixStrategy
15+
extends io.codemodder.remediation.MatchAndFixStrategy {
16+
17+
/**
18+
* Matches (XmlInputFactory | var xif) y = XMLInputFactory.newInstance(), where the node is the
19+
* newDocumentBuilder call.
20+
*
21+
* @param node the node to match
22+
* @return true if this is an assignment to an XMLInputFactory
23+
*/
24+
@Override
25+
public boolean match(final Node node) {
26+
Optional<MethodCallExpr> method =
27+
ASTs.isInitializedToType(node, "newInstance", List.of("var", "XMLInputFactory"));
28+
if (method.isEmpty()) {
29+
return false;
30+
}
31+
MethodCallExpr newInstance = method.get();
32+
Optional<Expression> scope = newInstance.getScope();
33+
if (scope.isEmpty()) {
34+
return false;
35+
}
36+
return scope.get().toString().equals("XMLInputFactory");
37+
}
38+
39+
@Override
40+
public SuccessOrReason fix(final CompilationUnit cu, final Node node) {
41+
MethodCallExpr newInstance = (MethodCallExpr) node;
42+
Optional<VariableDeclarator> initExpr = ASTs.isInitExpr(newInstance);
43+
if (initExpr.isEmpty()) {
44+
return SuccessOrReason.reason("Not an initialization expression");
45+
}
46+
VariableDeclarator xmlInputFactoryVariable = initExpr.get();
47+
Optional<Statement> variableDeclarationStmtRef =
48+
xmlInputFactoryVariable.findAncestor(Statement.class);
49+
50+
if (variableDeclarationStmtRef.isEmpty()) {
51+
return SuccessOrReason.reason("Not assigned as part of statement");
52+
}
53+
54+
Statement stmt = variableDeclarationStmtRef.get();
55+
return XMLFixBuilder.addXMLInputFactoryDisablingStatement(
56+
xmlInputFactoryVariable.getNameAsExpression(), stmt, false);
57+
}
58+
}
Collapse file

‎…ermediateXMLStreamReaderFixStrategy.java‎ ‎…StreamReaderIntermediateFixStrategy.java‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XXEIntermediateXMLStreamReaderFixStrategy.java renamed to framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XMLStreamReaderIntermediateFixStrategy.java framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XXEIntermediateXMLStreamReaderFixStrategy.java renamed to framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XMLStreamReaderIntermediateFixStrategy.java

Copy file name to clipboardExpand all lines: framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XMLStreamReaderIntermediateFixStrategy.java
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* Fix strategy for XXE vulnerabilities anchored to arguments of a XMLStreamReader calls. Finds the
1616
* parser's declaration and add statements disabling external entities and features.
1717
*/
18-
final class XXEIntermediateXMLStreamReaderFixStrategy implements RemediationStrategy {
18+
final class XMLStreamReaderIntermediateFixStrategy implements RemediationStrategy {
1919

2020
@Override
2121
public SuccessOrReason fix(final CompilationUnit cu, final Node node) {
@@ -27,7 +27,7 @@ public SuccessOrReason fix(final CompilationUnit cu, final Node node) {
2727
if (maybeCall.isEmpty()) {
2828
return SuccessOrReason.reason("Not a method call");
2929
}
30-
// get the xmlstreamreader scope variable
30+
// get the XMLStreamReader scope variable
3131
MethodCallExpr createXMLStreamReaderCall = maybeCall.get();
3232
Expression xmlStreamReaderScope = createXMLStreamReaderCall.getScope().get();
3333

Collapse file

‎…termediateXMLStreamReaderRemediator.java‎ ‎…LStreamReaderIntermediateRemediator.java‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XXEIntermediateXMLStreamReaderRemediator.java renamed to framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XMLStreamReaderIntermediateRemediator.java framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XXEIntermediateXMLStreamReaderRemediator.java renamed to framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XMLStreamReaderIntermediateRemediator.java

Copy file name to clipboardExpand all lines: framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XMLStreamReaderIntermediateRemediator.java
+12-11Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
import java.util.Optional;
1414
import java.util.function.Function;
1515

16-
public class XXEIntermediateXMLStreamReaderRemediator<T> implements Remediator<T> {
16+
/** Remediator for XXE vulnerabilities anchored to the XMLStreamReader calls. */
17+
public final class XMLStreamReaderIntermediateRemediator<T> implements Remediator<T> {
1718

1819
private final SearcherStrategyRemediator<T> searchStrategyRemediator;
1920

20-
public XXEIntermediateXMLStreamReaderRemediator() {
21+
public XMLStreamReaderIntermediateRemediator() {
2122
this.searchStrategyRemediator =
2223
new SearcherStrategyRemediator.Builder<T>()
2324
.withSearcherStrategyPair(
@@ -30,20 +31,20 @@ public XXEIntermediateXMLStreamReaderRemediator() {
3031
.filter(Node::hasScope)
3132
.isPresent())
3233
.build(),
33-
new XXEIntermediateXMLStreamReaderFixStrategy())
34+
new XMLStreamReaderIntermediateFixStrategy())
3435
.build();
3536
}
3637

3738
@Override
3839
public CodemodFileScanningResult remediateAll(
39-
CompilationUnit cu,
40-
String path,
41-
DetectorRule detectorRule,
42-
Collection<T> findingsForPath,
43-
Function<T, String> findingIdExtractor,
44-
Function<T, Integer> findingStartLineExtractor,
45-
Function<T, Optional<Integer>> findingEndLineExtractor,
46-
Function<T, Optional<Integer>> findingColumnExtractor) {
40+
final CompilationUnit cu,
41+
final String path,
42+
final DetectorRule detectorRule,
43+
final Collection<T> findingsForPath,
44+
final Function<T, String> findingIdExtractor,
45+
final Function<T, Integer> findingStartLineExtractor,
46+
final Function<T, Optional<Integer>> findingEndLineExtractor,
47+
final Function<T, Optional<Integer>> findingColumnExtractor) {
4748
return searchStrategyRemediator.remediateAll(
4849
cu,
4950
path,
Collapse file

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XXERemediator.java‎

Copy file name to clipboardExpand all lines: framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/XXERemediator.java
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ public XXERemediator(final NodePositionMatcher matcher) {
2929
.withMatchAndFixStrategyAndNodeMatcher(
3030
new TransformerFactoryAtCreationFixStrategy(), matcher)
3131
.withMatchAndFixStrategyAndNodeMatcher(new XMLReaderAtParseFixStrategy(), matcher)
32+
.withMatchAndFixStrategyAndNodeMatcher(
33+
new XMLInputFactoryAtNewInstanceFixStrategy(), matcher)
3234
.build();
3335
}
3436

Collapse file

‎…ediateXMLStreamReaderRemediatorTest.java‎ ‎…eamReaderIntermediateRemediatorTest.java‎framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/DefaultXXEIntermediateXMLStreamReaderRemediatorTest.java renamed to framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/DefaultXMLStreamReaderIntermediateRemediatorTest.java framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/DefaultXXEIntermediateXMLStreamReaderRemediatorTest.java renamed to framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/DefaultXMLStreamReaderIntermediateRemediatorTest.java

Copy file name to clipboardExpand all lines: framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/DefaultXMLStreamReaderIntermediateRemediatorTest.java
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@
1414
import org.junit.jupiter.api.BeforeEach;
1515
import org.junit.jupiter.api.Test;
1616

17-
final class DefaultXXEIntermediateXMLStreamReaderRemediatorTest {
17+
final class DefaultXMLStreamReaderIntermediateRemediatorTest {
1818

19-
private XXEIntermediateXMLStreamReaderRemediator<Object> remediator;
19+
private XMLStreamReaderIntermediateRemediator<Object> remediator;
2020
private DetectorRule rule;
2121

2222
@BeforeEach
2323
void setup() {
24-
remediator = new XXEIntermediateXMLStreamReaderRemediator<>();
24+
remediator = new XMLStreamReaderIntermediateRemediator<>();
2525
rule = new DetectorRule("xxe", "XXE Fixed At XMLStreamReader", null);
2626
}
2727

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.