Skip to content

Commit ec403a7

Browse files
authored
Remove explicit setEntityExpansion calls (#394)
When fixing XXE, users may find it helpful to also remove explicit turning on off entity expansion.
1 parent adb461a commit ec403a7

File tree

5 files changed

+21
-9
lines changed

5 files changed

+21
-9
lines changed

core-codemods/src/test/resources/sonar-xxe-s2755/Test.java.after

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ public class XXEVuln {
7272
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
7373
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
7474
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
75-
dbf.setExpandEntityReferences(true);
7675
DocumentBuilder db = dbf.newDocumentBuilder();
7776
return db.parse(new InputSource(new StringReader(xml)));
7877
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,6 @@ public XXEFixAttempt tryFix(final int line, final Integer column, CompilationUni
4444

4545
Statement statement = variableDeclarationStmtRef.get();
4646
return addFeatureDisablingStatements(
47-
cu, newFactoryVariable.getNameAsExpression(), statement, false);
47+
newFactoryVariable.getNameAsExpression(), statement, false);
4848
}
4949
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public XXEFixAttempt tryFix(final int line, final Integer column, CompilationUni
104104
"DocumentBuilder was initialized with a factory call without a statement");
105105
}
106106
return XMLFeatures.addFeatureDisablingStatements(
107-
cu, factoryNameExpr, newDocumentBuilderStatement.get(), true);
107+
factoryNameExpr, newDocumentBuilderStatement.get(), true);
108108
} else if (parserAssignmentNode instanceof Parameter) {
109109
return new XXEFixAttempt(true, false, "DocumentBuilder came from outside the method scope");
110110
}

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package io.codemodder.remediation.xxe;
22

3-
import com.github.javaparser.ast.CompilationUnit;
3+
import com.github.javaparser.ast.Node;
44
import com.github.javaparser.ast.NodeList;
55
import com.github.javaparser.ast.expr.BooleanLiteralExpr;
66
import com.github.javaparser.ast.expr.MethodCallExpr;
@@ -39,10 +39,7 @@ static MethodCallExpr createGeneralEntityDisablingCall(final NameExpr nameExpr)
3939
}
4040

4141
static XXEFixAttempt addFeatureDisablingStatements(
42-
final CompilationUnit cu,
43-
final NameExpr variable,
44-
final Statement statementToInjectAround,
45-
final boolean before) {
42+
final NameExpr variable, final Statement statementToInjectAround, final boolean before) {
4643
Optional<BlockStmt> block = ASTs.findBlockStatementFrom(statementToInjectAround);
4744
if (block.isEmpty()) {
4845
return new XXEFixAttempt(true, false, "No block statement found for newFactory() call");
@@ -62,6 +59,22 @@ static XXEFixAttempt addFeatureDisablingStatements(
6259
index++;
6360
}
6461
existingStatements.addAll(index, fixStatements);
62+
63+
// search for any dbf.setExpandEntityReferences(true) calls and remove them
64+
blockStmt.findAll(MethodCallExpr.class).stream()
65+
.filter(
66+
m ->
67+
"setExpandEntityReferences".equals(m.getNameAsString())
68+
&& m.getScope().isPresent()
69+
&& m.getScope().get().equals(variable)
70+
&& m.getArguments().size() == 1
71+
&& m.getArguments().get(0).isBooleanLiteralExpr()
72+
&& m.getArguments().get(0).asBooleanLiteralExpr().getValue())
73+
.map(Node::getParentNode)
74+
.filter(Optional::isPresent)
75+
.map(Optional::get)
76+
.forEach(Node::remove);
77+
6578
return new XXEFixAttempt(true, true, null);
6679
}
6780
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,6 @@ public XXEFixAttempt tryFix(final int line, final Integer column, final Compilat
7272
return new XXEFixAttempt(true, false, "No statement found for parse() call");
7373
}
7474
return XMLFeatures.addFeatureDisablingStatements(
75-
cu, parser.asNameExpr(), parseStatement.get(), true);
75+
parser.asNameExpr(), parseStatement.get(), true);
7676
}
7777
}

0 commit comments

Comments
 (0)