C#: Re-refactor Xss to use the new API.

This commit is contained in:
Michael Nebel
2023-04-17 13:42:57 +02:00
parent 94e0828ab9
commit 0fdeeba46f
2 changed files with 53 additions and 23 deletions

View File

@@ -18,12 +18,10 @@ private import semmle.code.csharp.dataflow.TaintTracking2
*/
predicate xssFlow(XssNode source, XssNode sink, string message) {
// standard taint-tracking
exists(
TaintTrackingConfiguration c, DataFlow2::PathNode sourceNode, DataFlow2::PathNode sinkNode
|
exists(XssTracking::PathNode sourceNode, XssTracking::PathNode sinkNode |
sourceNode = source.asDataFlowNode() and
sinkNode = sink.asDataFlowNode() and
c.hasFlowPath(sourceNode, sinkNode) and
XssTracking::flowPath(sourceNode, sinkNode) and
message =
"is written to HTML or JavaScript" +
any(string explanation |
@@ -45,7 +43,7 @@ predicate xssFlow(XssNode source, XssNode sink, string message) {
module PathGraph {
/** Holds if `(pred,succ)` is an edge in the graph of data flow path explanations. */
query predicate edges(XssNode pred, XssNode succ) {
exists(DataFlow2::PathNode a, DataFlow2::PathNode b | DataFlow2::PathGraph::edges(a, b) |
exists(XssTracking::PathNode a, XssTracking::PathNode b | XssTracking::PathGraph::edges(a, b) |
pred.asDataFlowNode() = a and
succ.asDataFlowNode() = b
)
@@ -56,7 +54,7 @@ module PathGraph {
/** Holds if `n` is a node in the graph of data flow path explanations. */
query predicate nodes(XssNode n, string key, string val) {
DataFlow2::PathGraph::nodes(n.asDataFlowNode(), key, val)
XssTracking::PathGraph::nodes(n.asDataFlowNode(), key, val)
or
xssFlow(n, n, _) and
key = "semmle.label" and
@@ -69,13 +67,13 @@ module PathGraph {
* `ret -> out` is summarized as the edge `arg -> out`.
*/
query predicate subpaths(XssNode arg, XssNode par, XssNode ret, XssNode out) {
DataFlow2::PathGraph::subpaths(arg.asDataFlowNode(), par.asDataFlowNode(), ret.asDataFlowNode(),
out.asDataFlowNode())
XssTracking::PathGraph::subpaths(arg.asDataFlowNode(), par.asDataFlowNode(),
ret.asDataFlowNode(), out.asDataFlowNode())
}
}
private newtype TXssNode =
TXssDataFlowNode(DataFlow2::PathNode node) or
TXssDataFlowNode(XssTracking::PathNode node) or
TXssAspNode(AspInlineMember m)
/**
@@ -90,21 +88,25 @@ class XssNode extends TXssNode {
/** Gets the location of this node. */
Location getLocation() { none() }
/** Gets the data flow node corresponding to this node, if any. */
DataFlow2::PathNode asDataFlowNode() { result = this.(XssDataFlowNode).getDataFlowNode() }
/**
* Gets the data flow node corresponding to this node, if any.
*/
XssTracking::PathNode asDataFlowNode() { result = this.(XssDataFlowNode).getDataFlowNode() }
/** Gets the ASP inline code element corresponding to this node, if any. */
AspInlineMember asAspInlineMember() { result = this.(XssAspNode).getAspInlineMember() }
}
/** A data flow node, viewed as an XSS flow node. */
/**
* A data flow node, viewed as an XSS flow node.
*/
class XssDataFlowNode extends TXssDataFlowNode, XssNode {
DataFlow2::PathNode node;
XssTracking::PathNode node;
XssDataFlowNode() { this = TXssDataFlowNode(node) }
/** Gets the data flow node corresponding to this node. */
DataFlow2::PathNode getDataFlowNode() { result = node }
XssTracking::PathNode getDataFlowNode() { result = node }
override string toString() { result = node.toString() }
@@ -136,9 +138,11 @@ abstract class Source extends DataFlow::Node { }
abstract class Sanitizer extends DataFlow::ExprNode { }
/**
* DEPRECATED: Use `XssTracking` instead.
*
* A taint-tracking configuration for cross-site scripting (XSS) vulnerabilities.
*/
class TaintTrackingConfiguration extends TaintTracking2::Configuration {
deprecated class TaintTrackingConfiguration extends TaintTracking2::Configuration {
TaintTrackingConfiguration() { this = "XSSDataFlowConfiguration" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -148,6 +152,29 @@ class TaintTrackingConfiguration extends TaintTracking2::Configuration {
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
}
/**
* A taint-tracking configuration for cross-site scripting (XSS) vulnerabilities.
*/
module XssTrackingConfig implements DataFlow::ConfigSig {
/**
* Holds if `source` is a relevant data flow source.
*/
predicate isSource(DataFlow::Node source) { source instanceof Source }
/**
* Holds if `sink` is a relevant data flow sink.
*/
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
/**
* Holds if data flow through `node` is prohibited. This completely removes
* `node` from the data flow graph.
*/
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
}
module XssTracking = TaintTracking::Global<XssTrackingConfig>;
/** A source of remote user input. */
private class RemoteSource extends Source instanceof RemoteFlowSource { }

View File

@@ -16,18 +16,21 @@ import csharp
import semmle.code.csharp.security.dataflow.flowsources.Stored
import semmle.code.csharp.security.dataflow.XSSQuery
import semmle.code.csharp.security.dataflow.XSSSinks
import semmle.code.csharp.dataflow.DataFlow2
import DataFlow2::PathGraph
import StoredXss::PathGraph
class StoredTaintTrackingConfiguration extends TaintTrackingConfiguration {
override predicate isSource(DataFlow2::Node source) { source instanceof StoredFlowSource }
module StoredXssTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof StoredFlowSource }
predicate isSink = XssTrackingConfig::isSink/1;
predicate isBarrier = XssTrackingConfig::isBarrier/1;
}
from
StoredTaintTrackingConfiguration c, DataFlow2::PathNode source, DataFlow2::PathNode sink,
string explanation
module StoredXss = TaintTracking::Global<StoredXssTrackingConfig>;
from StoredXss::PathNode source, StoredXss::PathNode sink, string explanation
where
c.hasFlowPath(source, sink) and
StoredXss::flowPath(source, sink) and
if exists(sink.getNode().(Sink).explanation())
then explanation = " (" + sink.getNode().(Sink).explanation() + ")"
else explanation = ""