Skip to content

Commit 3ea92ea

Browse files
authored
Merge pull request #21024 from MathiasVP/csharp-implicit-map-value-reads
C#: Add implicit `System.Collections.Generic.KeyValuePair2.Value` reads at taint sinks
2 parents b61a439 + 2720f57 commit 3ea92ea

File tree

8 files changed

+2271
-13
lines changed

8 files changed

+2271
-13
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added implicit reads of `System.Collections.Generic.KeyValuePair.Value` at taint-tracking sinks and at inputs to additional taint steps. As a result, taint-tracking queries will now produce more results when a container is tainted.

csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,25 @@ predicate defaultTaintSanitizer(DataFlow::Node node) {
2424
)
2525
}
2626

27+
/**
28+
* Gets the (unbound) property `System.Collections.Generic.KeyValuePair.Value`.
29+
*/
30+
private Property keyValuePairValue() {
31+
result.hasFullyQualifiedName("System.Collections.Generic", "KeyValuePair`2", "Value")
32+
}
33+
2734
/**
2835
* Holds if default `TaintTracking::Configuration`s should allow implicit reads
2936
* of `c` at sinks and inputs to additional taint steps.
3037
*/
3138
bindingset[node]
3239
predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) {
3340
exists(node) and
34-
c.isElement()
41+
(
42+
c.isElement()
43+
or
44+
c.isProperty(keyValuePairValue())
45+
)
3546
}
3647

3748
private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityConfiguration {

csharp/ql/test/library-tests/dataflow/collections/CollectionDataFlow.expected

Lines changed: 840 additions & 0 deletions
Large diffs are not rendered by default.

csharp/ql/test/library-tests/dataflow/collections/CollectionFlow.ql renamed to csharp/ql/test/library-tests/dataflow/collections/CollectionDataFlow.ql

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,9 @@
22
* @kind path-problem
33
*/
44

5-
import csharp
5+
import CollectionFlowCommon
66
import utils.test.ProvenancePathGraph::ShowProvenance<ArrayFlow::PathNode, ArrayFlow::PathGraph>
77

8-
module ArrayFlowConfig implements DataFlow::ConfigSig {
9-
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ObjectCreation }
10-
11-
predicate isSink(DataFlow::Node sink) {
12-
exists(MethodCall mc |
13-
mc.getTarget().hasUndecoratedName("Sink") and
14-
mc.getAnArgument() = sink.asExpr()
15-
)
16-
}
17-
}
18-
198
module ArrayFlow = DataFlow::Global<ArrayFlowConfig>;
209

2110
from ArrayFlow::PathNode source, ArrayFlow::PathNode sink

csharp/ql/test/library-tests/dataflow/collections/CollectionFlow.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,4 +550,10 @@ public void ReadOnlySpanConstructorFlow()
550550
ReadOnlySpan<A> span = new ReadOnlySpan<A>(new[] { a });
551551
Sink(span[0]); // flow
552552
}
553+
554+
public void ImplicitMapValueRead(Dictionary<int, A> dict) {
555+
var a = new A();
556+
dict[0] = a;
557+
Sink(dict); // taint flow
558+
}
553559
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import csharp
2+
3+
module ArrayFlowConfig implements DataFlow::ConfigSig {
4+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ObjectCreation }
5+
6+
predicate isSink(DataFlow::Node sink) {
7+
exists(MethodCall mc |
8+
mc.getTarget().hasUndecoratedName("Sink") and
9+
mc.getAnArgument() = sink.asExpr()
10+
)
11+
}
12+
}

csharp/ql/test/library-tests/dataflow/collections/CollectionTaintFlow.expected

Lines changed: 1384 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import CollectionFlowCommon
6+
import utils.test.ProvenancePathGraph::ShowProvenance<ArrayFlow::PathNode, ArrayFlow::PathGraph>
7+
8+
module ArrayFlow = TaintTracking::Global<ArrayFlowConfig>;
9+
10+
from ArrayFlow::PathNode source, ArrayFlow::PathNode sink
11+
where ArrayFlow::flowPath(source, sink)
12+
select source, source, sink, "$@", sink, sink.toString()

0 commit comments

Comments
 (0)