Skip to content

Commit c89f2e3

Browse files
michaelnebelmichaelnebel
authored
Merge pull request #20089 from michaelnebel/csharp/allowsinkimplicitread
C#: Allow implicit collection reads in sink nodes.
2 parents 771d7cb + ebfbc71 commit c89f2e3

File tree

9 files changed

+69
-8
lines changed

9 files changed

+69
-8
lines changed

‎csharp/ql/lib/change-notes/2025-08-18-implicit-reads-at-sinks.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The default taint tracking configuration now allows implicit reads from collections at sinks and in additional flow steps. This increases flow coverage for many taint tracking queries and helps reduce false negatives.

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ predicate defaultTaintSanitizer(DataFlow::Node node) {
2929
* of `c` at sinks and inputs to additional taint steps.
3030
*/
3131
bindingset[node]
32-
predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) { none() }
32+
predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) {
33+
exists(node) and
34+
c.isElement()
35+
}
3336

3437
private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityConfiguration {
3538
LocalTaintExprStepConfiguration() { this = "LocalTaintExprStepConfiguration" }

‎csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.ql

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*/
1111

1212
import csharp
13+
import semmle.code.csharp.frameworks.system.Collections
1314
import HashWithoutSalt::PathGraph
1415

1516
/** The C# class `Windows.Security.Cryptography.Core.HashAlgorithmProvider`. */
@@ -93,12 +94,17 @@ predicate hasAnotherHashCall(MethodCall mc) {
9394

9495
/** Holds if a password hash without salt is further processed in another method call. */
9596
predicate hasFurtherProcessing(MethodCall mc) {
96-
mc.getTarget().fromLibrary() and
97-
(
98-
mc.getTarget().hasFullyQualifiedName("System", "Array", "Copy") or // Array.Copy(passwordHash, 0, password.Length), 0, key, 0, keyLen);
99-
mc.getTarget().hasFullyQualifiedName("System", "String", "Concat") or // string.Concat(passwordHash, saltkey)
100-
mc.getTarget().hasFullyQualifiedName("System", "Buffer", "BlockCopy") or // Buffer.BlockCopy(passwordHash, 0, allBytes, 0, 20)
101-
mc.getTarget().hasFullyQualifiedName("System", "String", "Format") // String.Format("{0}:{1}:{2}", username, salt, password)
97+
exists(Method m | m = mc.getTarget() and m.fromLibrary() |
98+
m.hasFullyQualifiedName("System", "Array", "Copy") // Array.Copy(passwordHash, 0, password.Length), 0, key, 0, keyLen);
99+
or
100+
m.hasFullyQualifiedName("System", "String", "Concat") // string.Concat(passwordHash, saltkey)
101+
or
102+
m.hasFullyQualifiedName("System", "Buffer", "BlockCopy") // Buffer.BlockCopy(passwordHash, 0, allBytes, 0, 20)
103+
or
104+
m.hasFullyQualifiedName("System", "String", "Format") // String.Format("{0}:{1}:{2}", username, salt, password)
105+
or
106+
m.getName() = "CopyTo" and
107+
m.getDeclaringType().getABaseType*() instanceof SystemCollectionsICollectionInterface // passBytes.CopyTo(rawSalted, 0);
102108
)
103109
}
104110

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ void M17()
116116
{
117117
var a = new object[] { new object() };
118118
var b = Reverse(a);
119-
Sink(b); // No flow
119+
Sink(b); // Flow
120120
Sink(b[0]); // Flow
121121
}
122122

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ edges
104104
| ExternalFlow.cs:117:17:117:17 | access to local variable a : null [element] : Object | ExternalFlow.cs:118:29:118:29 | access to local variable a : null [element] : Object | provenance | |
105105
| ExternalFlow.cs:117:34:117:49 | { ..., ... } : null [element] : Object | ExternalFlow.cs:117:17:117:17 | access to local variable a : null [element] : Object | provenance | |
106106
| ExternalFlow.cs:117:36:117:47 | object creation of type Object : Object | ExternalFlow.cs:117:34:117:49 | { ..., ... } : null [element] : Object | provenance | |
107+
| ExternalFlow.cs:118:17:118:17 | access to local variable b : null [element] : Object | ExternalFlow.cs:119:18:119:18 | access to local variable b | provenance | |
107108
| ExternalFlow.cs:118:17:118:17 | access to local variable b : null [element] : Object | ExternalFlow.cs:120:18:120:18 | access to local variable b : null [element] : Object | provenance | |
108109
| ExternalFlow.cs:118:21:118:30 | call to method Reverse : null [element] : Object | ExternalFlow.cs:118:17:118:17 | access to local variable b : null [element] : Object | provenance | |
109110
| ExternalFlow.cs:118:29:118:29 | access to local variable a : null [element] : Object | ExternalFlow.cs:118:21:118:30 | call to method Reverse : null [element] : Object | provenance | MaD:7 |
@@ -240,6 +241,7 @@ nodes
240241
| ExternalFlow.cs:118:17:118:17 | access to local variable b : null [element] : Object | semmle.label | access to local variable b : null [element] : Object |
241242
| ExternalFlow.cs:118:21:118:30 | call to method Reverse : null [element] : Object | semmle.label | call to method Reverse : null [element] : Object |
242243
| ExternalFlow.cs:118:29:118:29 | access to local variable a : null [element] : Object | semmle.label | access to local variable a : null [element] : Object |
244+
| ExternalFlow.cs:119:18:119:18 | access to local variable b | semmle.label | access to local variable b |
243245
| ExternalFlow.cs:120:18:120:18 | access to local variable b : null [element] : Object | semmle.label | access to local variable b : null [element] : Object |
244246
| ExternalFlow.cs:120:18:120:21 | access to array element | semmle.label | access to array element |
245247
| ExternalFlow.cs:205:17:205:18 | access to local variable o2 : Object | semmle.label | access to local variable o2 : Object |
@@ -315,6 +317,7 @@ invalidModelRow
315317
| ExternalFlow.cs:102:22:102:22 | access to parameter d | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:102:22:102:22 | access to parameter d | $@ | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | object creation of type Object : Object |
316318
| ExternalFlow.cs:104:18:104:25 | access to field Field | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:104:18:104:25 | access to field Field | $@ | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | object creation of type Object : Object |
317319
| ExternalFlow.cs:112:18:112:25 | access to property MyProp | ExternalFlow.cs:111:24:111:35 | object creation of type Object : Object | ExternalFlow.cs:112:18:112:25 | access to property MyProp | $@ | ExternalFlow.cs:111:24:111:35 | object creation of type Object : Object | object creation of type Object : Object |
320+
| ExternalFlow.cs:119:18:119:18 | access to local variable b | ExternalFlow.cs:117:36:117:47 | object creation of type Object : Object | ExternalFlow.cs:119:18:119:18 | access to local variable b | $@ | ExternalFlow.cs:117:36:117:47 | object creation of type Object : Object | object creation of type Object : Object |
318321
| ExternalFlow.cs:120:18:120:21 | access to array element | ExternalFlow.cs:117:36:117:47 | object creation of type Object : Object | ExternalFlow.cs:120:18:120:21 | access to array element | $@ | ExternalFlow.cs:117:36:117:47 | object creation of type Object : Object | object creation of type Object : Object |
319322
| ExternalFlow.cs:206:18:206:48 | call to method MixedFlowArgs | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | ExternalFlow.cs:206:18:206:48 | call to method MixedFlowArgs | $@ | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | object creation of type Object : Object |
320323
| ExternalFlow.cs:212:18:212:62 | call to method GeneratedFlowWithGeneratedNeutral | ExternalFlow.cs:211:22:211:33 | object creation of type Object : Object | ExternalFlow.cs:212:18:212:62 | call to method GeneratedFlowWithGeneratedNeutral | $@ | ExternalFlow.cs:211:22:211:33 | object creation of type Object : Object | object creation of type Object : Object |

‎csharp/ql/test/library-tests/tainttracking/collections/CollectionTaintTracking.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
public class CollectionTaintTracking
2+
{
3+
public void ImplicitCollectionReadAtSink()
4+
{
5+
var tainted = Source<object>(1);
6+
var arr = new object[] { tainted };
7+
Sink(arr); // $ hasTaintFlow=1
8+
}
9+
10+
static T Source<T>(object source) => throw null;
11+
12+
public static void Sink<T>(T t) { }
13+
}

‎csharp/ql/test/library-tests/tainttracking/collections/CollectionTaintTracking.expected

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
models
2+
edges
3+
| CollectionTaintTracking.cs:5:13:5:19 | access to local variable tainted : Object | CollectionTaintTracking.cs:6:34:6:40 | access to local variable tainted : Object | provenance | |
4+
| CollectionTaintTracking.cs:5:23:5:39 | call to method Source<Object> : Object | CollectionTaintTracking.cs:5:13:5:19 | access to local variable tainted : Object | provenance | |
5+
| CollectionTaintTracking.cs:6:13:6:15 | access to local variable arr : null [element] : Object | CollectionTaintTracking.cs:7:14:7:16 | access to local variable arr | provenance | |
6+
| CollectionTaintTracking.cs:6:32:6:42 | { ..., ... } : null [element] : Object | CollectionTaintTracking.cs:6:13:6:15 | access to local variable arr : null [element] : Object | provenance | |
7+
| CollectionTaintTracking.cs:6:34:6:40 | access to local variable tainted : Object | CollectionTaintTracking.cs:6:32:6:42 | { ..., ... } : null [element] : Object | provenance | |
8+
nodes
9+
| CollectionTaintTracking.cs:5:13:5:19 | access to local variable tainted : Object | semmle.label | access to local variable tainted : Object |
10+
| CollectionTaintTracking.cs:5:23:5:39 | call to method Source<Object> : Object | semmle.label | call to method Source<Object> : Object |
11+
| CollectionTaintTracking.cs:6:13:6:15 | access to local variable arr : null [element] : Object | semmle.label | access to local variable arr : null [element] : Object |
12+
| CollectionTaintTracking.cs:6:32:6:42 | { ..., ... } : null [element] : Object | semmle.label | { ..., ... } : null [element] : Object |
13+
| CollectionTaintTracking.cs:6:34:6:40 | access to local variable tainted : Object | semmle.label | access to local variable tainted : Object |
14+
| CollectionTaintTracking.cs:7:14:7:16 | access to local variable arr | semmle.label | access to local variable arr |
15+
subpaths
16+
testFailures
17+
#select
18+
| CollectionTaintTracking.cs:7:14:7:16 | access to local variable arr | CollectionTaintTracking.cs:5:23:5:39 | call to method Source<Object> : Object | CollectionTaintTracking.cs:7:14:7:16 | access to local variable arr | $@ | CollectionTaintTracking.cs:5:23:5:39 | call to method Source<Object> : Object | call to method Source<Object> : Object |

‎csharp/ql/test/library-tests/tainttracking/collections/CollectionTaintTracking.ql

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 csharp
6+
import utils.test.InlineFlowTest
7+
import TaintFlowTest<DefaultFlowConfig>
8+
import PathGraph
9+
10+
from PathNode source, PathNode sink
11+
where flowPath(source, sink)
12+
select sink, source, sink, "$@", source, source.toString()

‎csharp/ql/test/library-tests/tainttracking/collections/options

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
semmle-extractor-options: /nostdlib /noconfig
2+
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj

0 commit comments

Comments
 (0)