Skip to content

Commit 89bdcfb

Browse files
committed
C#: Allow 'StartsWith' and 'EndsWith' to be barriers when the path is normalized.
1 parent 9457e53 commit 89bdcfb

File tree

1 file changed

+23
-7
lines changed

1 file changed

+23
-7
lines changed

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ abstract class Sink extends ApiSinkExprNode { }
2525
/**
2626
* A sanitizer for uncontrolled data in path expression vulnerabilities.
2727
*/
28-
abstract class Sanitizer extends DataFlow::ExprNode { }
28+
abstract class Sanitizer extends DataFlow::ExprNode {
29+
/** Holds if this is a sanitizer when the flow state is `state`. */
30+
predicate isBarrier(TaintedPathConfig::FlowState state) { any() }
31+
}
2932

3033
/** A path normalization step. */
3134
private class PathNormalizationStep extends Unit {
@@ -141,7 +144,7 @@ class StreamWriterTaintedPathSink extends Sink {
141144
}
142145

143146
/**
144-
* A weak guard that is insufficient to prevent path tampering.
147+
* A weak guard that may be insufficient to prevent path tampering.
145148
*/
146149
private class WeakGuard extends Guard {
147150
WeakGuard() {
@@ -160,6 +163,14 @@ private class WeakGuard extends Guard {
160163
or
161164
this.(LogicalOperation).getAnOperand() instanceof WeakGuard
162165
}
166+
167+
predicate isBarrier(TaintedPathConfig::FlowState state) {
168+
state = TaintedPathConfig::Normalized() and
169+
exists(Method m | this.(MethodCall).getTarget() = m |
170+
m.getName() = "StartsWith" or
171+
m.getName() = "EndsWith"
172+
)
173+
}
163174
}
164175

165176
/**
@@ -168,12 +179,17 @@ private class WeakGuard extends Guard {
168179
* A weak check is one that is insufficient to prevent path tampering.
169180
*/
170181
class PathCheck extends Sanitizer {
182+
Guard g;
183+
171184
PathCheck() {
172-
// This expression is structurally replicated in a dominating guard which is not a "weak" check
173-
exists(Guard g, AbstractValues::BooleanValue v |
174-
g = this.(GuardedDataFlowNode).getAGuard(_, v) and
175-
not g instanceof WeakGuard
176-
)
185+
// This expression is structurally replicated in a dominating guard
186+
exists(AbstractValues::BooleanValue v | g = this.(GuardedDataFlowNode).getAGuard(_, v))
187+
}
188+
189+
override predicate isBarrier(TaintedPathConfig::FlowState state) {
190+
g.(WeakGuard).isBarrier(state)
191+
or
192+
not g instanceof WeakGuard
177193
}
178194
}
179195

0 commit comments

Comments
 (0)