mirror of
https://github.com/github/codeql.git
synced 2026-01-08 20:20:34 +01:00
Merge pull request #1 from smowton/smowton/admin/rewrite-xquery
Rewrite XQuery injection to use an additional taint step instead of multiple configurations
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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() }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user