Consider second parameter of Node.selectNodes

This commit is contained in:
Tony Torralba
2021-04-27 15:49:49 +02:00
parent d72dd9b861
commit ab62bb66f4
2 changed files with 17 additions and 6 deletions

View File

@@ -18,6 +18,8 @@ private class XPathEvaluation extends MethodAccess {
m.hasName(["evaluate", "evaluateExpression", "compile"])
)
}
Expr getSink() { result = this.getArgument(0) }
}
/** The interface `org.dom4j.Node` */
@@ -27,16 +29,25 @@ private class Dom4JNode extends Interface {
/** A call to methods of any class implementing the interface `Node` that evaluate XPath expressions */
private class NodeXPathEvaluation extends MethodAccess {
Expr sink;
NodeXPathEvaluation() {
exists(Method m |
this.getMethod() = m and m.getDeclaringType().getASourceSupertype*() instanceof Dom4JNode
exists(Method m, int index |
this.getMethod() = m and
m.getDeclaringType().getASourceSupertype*() instanceof Dom4JNode and
sink = this.getArgument(index)
|
m.hasName([
"selectObject", "selectNodes", "selectSingleNode", "numberValueOf", "valueOf", "matches",
"createXPath"
])
]) and
index = 0
or
m.hasName("selectNodes") and index in [0, 1]
)
}
Expr getSink() { result = sink }
}
/**
@@ -47,7 +58,7 @@ abstract class XPathInjectionSink extends DataFlow::Node { }
private class DefaultXPathInjectionSink extends XPathInjectionSink {
DefaultXPathInjectionSink() {
exists(NodeXPathEvaluation sink | sink.getArgument(0) = this.asExpr()) or
exists(XPathEvaluation sink | sink.getArgument(0) = this.asExpr())
exists(NodeXPathEvaluation sink | sink.getSink() = this.asExpr()) or
exists(XPathEvaluation sink | sink.getSink() = this.asExpr())
}
}

View File

@@ -107,7 +107,7 @@ public class A {
org.dom4j.Document document = reader.read(new ByteArrayInputStream(xmlStr.getBytes()));
document.selectObject("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection
document.selectNodes("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection
document.selectNodes("/users/user[@name='test']", "/users/user[@pass='" + pass + "']"); // Safe-ish
document.selectNodes("/users/user[@name='test']", "/users/user[@pass='" + pass + "']"); // $hasXPathInjection
document.selectSingleNode("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection
document.valueOf("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection
document.numberValueOf("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection