Skip to content

Commit 9457e53

Browse files
committed
C#: Add a flow state to represent whether the path is normalized.
1 parent 864bde2 commit 9457e53

File tree

1 file changed

+47
-5
lines changed

1 file changed

+47
-5
lines changed

csharp/ql/lib/semmle/code/csharp/security/dataflow/TaintedPathQuery.qll

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import csharp
7+
private import codeql.util.Unit
78
private import semmle.code.csharp.controlflow.Guards
89
private import semmle.code.csharp.security.dataflow.flowsinks.FlowSinks
910
private import semmle.code.csharp.security.dataflow.flowsources.FlowSources
@@ -26,21 +27,62 @@ abstract class Sink extends ApiSinkExprNode { }
2627
*/
2728
abstract class Sanitizer extends DataFlow::ExprNode { }
2829

30+
/** A path normalization step. */
31+
private class PathNormalizationStep extends Unit {
32+
/**
33+
* Holds if the flow step from `n1` to `n2` transforms the path into an
34+
* absolute path.
35+
*
36+
* For example, the argument-to-return-value step through a call
37+
* to `System.IO.Path.GetFullPath` is a normalization step.
38+
*/
39+
abstract predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2);
40+
}
41+
42+
private class GetFullPathStep extends PathNormalizationStep {
43+
override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
44+
exists(Call call |
45+
call.getARuntimeTarget().hasFullyQualifiedName("System.IO.Path", "GetFullPath") and
46+
n1.asExpr() = call.getArgument(0) and
47+
n2.asExpr() = call
48+
)
49+
}
50+
}
51+
2952
/**
3053
* A taint-tracking configuration for uncontrolled data in path expression vulnerabilities.
3154
*/
32-
private module TaintedPathConfig implements DataFlow::ConfigSig {
33-
predicate isSource(DataFlow::Node source) { source instanceof Source }
55+
private module TaintedPathConfig implements DataFlow::StateConfigSig {
56+
newtype FlowState =
57+
additional NotNormalized() or
58+
additional Normalized()
3459

35-
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
60+
predicate isSource(DataFlow::Node source, FlowState state) {
61+
source instanceof Source and state = NotNormalized()
62+
}
3663

37-
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
64+
predicate isSink(DataFlow::Node sink, FlowState state) {
65+
sink instanceof Sink and
66+
exists(state)
67+
}
68+
69+
predicate isAdditionalFlowStep(DataFlow::Node n1, FlowState s1, DataFlow::Node n2, FlowState s2) {
70+
any(PathNormalizationStep step).isAdditionalFlowStep(n1, n2) and
71+
s1 = NotNormalized() and
72+
s2 = Normalized()
73+
}
74+
75+
predicate isBarrier(DataFlow::Node node, FlowState state) { node.(Sanitizer).isBarrier(state) }
76+
77+
predicate isBarrierOut(DataFlow::Node node, FlowState state) {
78+
isAdditionalFlowStep(node, state, _, _)
79+
}
3880
}
3981

4082
/**
4183
* A taint-tracking module for uncontrolled data in path expression vulnerabilities.
4284
*/
43-
module TaintedPath = TaintTracking::Global<TaintedPathConfig>;
85+
module TaintedPath = TaintTracking::GlobalWithState<TaintedPathConfig>;
4486

4587
/**
4688
* DEPRECATED: Use `ThreatModelSource` instead.

0 commit comments

Comments
 (0)