diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/NodePositionMatcher.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/NodePositionMatcher.java index 4c5c400b8..166e0abee 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/NodePositionMatcher.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/NodePositionMatcher.java @@ -5,7 +5,7 @@ /** Provides methods for matching nodes to given coordinates. */ public interface NodePositionMatcher { - static final NodePositionMatcher DEFAULT = new DefaultNodePositionMatcher(); + NodePositionMatcher DEFAULT = new DefaultNodePositionMatcher(); /** * Matches a node with a range against a line diff --git a/framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/DocumentBuilderFactoryAtParseFixStrategy.java b/framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/DocumentBuilderFactoryAtParseFixStrategy.java index 2313d1370..a795c748b 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/DocumentBuilderFactoryAtParseFixStrategy.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/remediation/xxe/DocumentBuilderFactoryAtParseFixStrategy.java @@ -101,6 +101,20 @@ public boolean match(final Node node) { .map(n -> n instanceof MethodCallExpr ? (MethodCallExpr) n : null) .filter(m -> "parse".equals(m.getNameAsString())) .filter(m -> m.getScope().filter(Expression::isNameExpr).isPresent()) + .filter( + m -> { + Optional sourceRef = + ASTs.findNonCallableSimpleNameSource(m.getScope().get().asNameExpr().getName()); + if (sourceRef.isEmpty()) { + return false; + } + Node source = sourceRef.get(); + if (source instanceof NodeWithType) { + return Set.of("DocumentBuilder", "javax.xml.parsers.DocumentBuilder") + .contains(((NodeWithType) source).getTypeAsString()); + } + return false; + }) .isPresent(); } } diff --git a/framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/XMLReaderAtParseFixerTest.java b/framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/XMLReaderAtParseFixerTest.java index 9b108b54f..791bb6e0d 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/XMLReaderAtParseFixerTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/remediation/xxe/XMLReaderAtParseFixerTest.java @@ -9,12 +9,38 @@ import io.codemodder.remediation.SearcherStrategyRemediator; import java.util.List; import java.util.Optional; -import org.junit.jupiter.api.Test; +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; final class XMLReaderAtParseFixerTest { - @Test - void it_fixes_xmlreaders_at_parse_call() { + private static Stream provideRemediators() { + return Stream.of( + Arguments.of( + new SearcherStrategyRemediator.Builder<>() + .withMatchAndFixStrategy(new XMLReaderAtParseFixStrategy()) + .build()), + + // now try with the previously conflicting parser strategy inline as well + Arguments.of( + new SearcherStrategyRemediator.Builder<>() + .withMatchAndFixStrategy(new DocumentBuilderFactoryAtParseFixStrategy()) + .withMatchAndFixStrategy(new XMLReaderAtParseFixStrategy()) + .build()), + + // reverse the order to confirm no ordering issues + Arguments.of( + new SearcherStrategyRemediator.Builder<>() + .withMatchAndFixStrategy(new XMLReaderAtParseFixStrategy()) + .withMatchAndFixStrategy(new DocumentBuilderFactoryAtParseFixStrategy()) + .build())); + } + + @MethodSource("provideRemediators") + @ParameterizedTest + void it_fixes_xmlreaders_at_parse_call(final SearcherStrategyRemediator remediator) { String vulnerableCode = """ public class MyCode { @@ -43,12 +69,8 @@ public void foo() { CompilationUnit cu = StaticJavaParser.parse(vulnerableCode); LexicalPreservingPrinter.setup(cu); - var searcherRemediator = - new SearcherStrategyRemediator.Builder<>() - .withMatchAndFixStrategy(new XMLReaderAtParseFixStrategy()) - .build(); var result = - searcherRemediator.remediateAll( + remediator.remediateAll( cu, "path", new DetectorRule("", "", ""),