diff --git a/java/ql/src/Security/CWE/CWE-652/XQueryInjection.ql b/java/ql/src/Security/CWE/CWE-652/XQueryInjection.ql index b4fcac80a43..7163a1fd93d 100644 --- a/java/ql/src/Security/CWE/CWE-652/XQueryInjection.ql +++ b/java/ql/src/Security/CWE/CWE-652/XQueryInjection.ql @@ -15,25 +15,21 @@ import semmle.code.java.dataflow.FlowSources import XQueryInjectionLib import DataFlow::PathGraph -class XQueryInjectionConfig extends DataFlow::Configuration { +class XQueryInjectionConfig extends TaintTracking::Configuration { XQueryInjectionConfig() { this = "XQueryInjectionConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof XQueryInjectionSource } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - override predicate isSink(DataFlow::Node sink) { sink instanceof XQueryInjectionSink } + override predicate isSink(DataFlow::Node sink) { + sink.asExpr() = any(XQueryExecuteCall execute).getPreparedExpression() + } - override predicate isBarrier(DataFlow::Node node) { - exists(MethodAccess ma, Method m, BindParameterRemoteFlowConf conf, DataFlow::Node node1 | - m = ma.getMethod() - | - node.asExpr() = ma and - m.hasName("bindString") and - m.getDeclaringType() - .getASourceSupertype*() - .hasQualifiedName("javax.xml.xquery", "XQDynamicContext") and - ma.getArgument(1) = node1.asExpr() and - conf.hasFlowTo(node1) - ) + /** + * Conveys taint from the input to a `prepareExpression` call to the returned prepared expression. + */ + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(XQueryParserCall parser | + pred.asExpr() = parser.getInput() and succ.asExpr() = parser) } } diff --git a/java/ql/src/Security/CWE/CWE-652/XQueryInjectionLib.qll b/java/ql/src/Security/CWE/CWE-652/XQueryInjectionLib.qll index 3d5770b5be6..cfdbaaa70da 100644 --- a/java/ql/src/Security/CWE/CWE-652/XQueryInjectionLib.qll +++ b/java/ql/src/Security/CWE/CWE-652/XQueryInjectionLib.qll @@ -1,7 +1,4 @@ import java -import semmle.code.java.dataflow.FlowSources -import semmle.code.java.dataflow.TaintTracking2 -import DataFlow::PathGraph /** A call to `XQConnection.prepareExpression`. */ class XQueryParserCall extends MethodAccess { @@ -14,77 +11,25 @@ class XQueryParserCall extends MethodAccess { m.hasName("prepareExpression") ) } - /** Returns the first parameter of the `bindString` method. */ + + /** + * Returns the first parameter of the `prepareExpression` method, which provides + * the string, stream or reader to be compiled into a prepared expression. + */ Expr getInput() { result = this.getArgument(0) } } -/** A call to `XQDynamicContext.bindString`. */ -class XQueryBindStringCall extends MethodAccess { - XQueryBindStringCall() { - exists(Method m | - this.getMethod() = m and - m.getDeclaringType() - .getASourceSupertype*() - .hasQualifiedName("javax.xml.xquery", "XQDynamicContext") and - m.hasName("bindString") - ) - } - /** Returns the second parameter of the `bindString` method. */ - Expr getInput() { result = this.getArgument(1) } -} - -/** Used to determine whether to call the `prepareExpression` method, and the first parameter value can be remotely controlled. */ -class ParserParameterRemoteFlowConf extends TaintTracking2::Configuration { - ParserParameterRemoteFlowConf() { this = "ParserParameterRemoteFlowConf" } - - override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { - exists(XQueryParserCall xqpc | xqpc.getSink() = sink.asExpr()) - } -} - -/** Used to determine whether to call the `bindString` method, and the second parameter value can be controlled remotely. */ -class BindParameterRemoteFlowConf extends TaintTracking2::Configuration { - BindParameterRemoteFlowConf() { this = "BindParameterRemoteFlowConf" } - - override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { - exists(XQueryBindStringCall xqbsc | xqbsc.getSink() = sink.asExpr()) - } -} - -/** - * A data flow source for XQuery injection vulnerability. - * 1. `prepareExpression` call as sink. - * 2. Determine whether the `var1` parameter of `prepareExpression` method can be controlled remotely. - */ -class XQueryInjectionSource extends DataFlow::ExprNode { - XQueryInjectionSource() { - exists(MethodAccess ma, Method m, ParserParameterRemoteFlowConf conf, DataFlow::Node node | - m = ma.getMethod() - | - m.hasName("prepareExpression") and - m.getDeclaringType() - .getASourceSupertype*() - .hasQualifiedName("javax.xml.xquery", "XQConnection") and - asExpr() = ma and - node.asExpr() = ma.getArgument(0) and - conf.hasFlowTo(node) - ) - } -} - -/** A data flow sink for XQuery injection vulnerability. */ -class XQueryInjectionSink extends DataFlow::Node { - XQueryInjectionSink() { - exists(MethodAccess ma, Method m | m = ma.getMethod() | - m.hasName("executeQuery") and - m.getDeclaringType() - .getASourceSupertype*() - .hasQualifiedName("javax.xml.xquery", "XQPreparedExpression") and - asExpr() = ma.getQualifier() +/** A call to `XQPreparedExpression.executeQuery`. */ +class XQueryExecuteCall extends MethodAccess { + XQueryExecuteCall() { + exists(Method m | this.getMethod() = m and + m.hasName("executeQuery") and + m.getDeclaringType() + .getASourceSupertype*() + .hasQualifiedName("javax.xml.xquery", "XQPreparedExpression") ) } + + /** Return this prepared expression. */ + Expr getPreparedExpression() { result = this.getQualifier() } }