C#: Allow implicit collection reads in sink nodes.

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

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* 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

@@ -7,6 +7,7 @@ private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
 private import semmle.code.csharp.dataflow.internal.ControlFlowReachability
 private import semmle.code.csharp.dispatch.Dispatch
 private import semmle.code.csharp.commons.ComparisonTest
+private import semmle.code.csharp.commons.Collections as Collections
 // import `TaintedMember` definitions from other files to avoid potential reevaluation
 private import semmle.code.csharp.frameworks.JsonNET
 private import semmle.code.csharp.frameworks.WCF
@@ -29,7 +30,11 @@ predicate defaultTaintSanitizer(DataFlow::Node node) {
  * of `c` at sinks and inputs to additional taint steps.
  */
 bindingset[node]
-predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) { none() }
+predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) {
+  node instanceof ArgumentNode and
+  Collections::isCollectionType(node.getType()) and
+  c.isElement()
+}
 
 private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityConfiguration {
   LocalTaintExprStepConfiguration() { this = "LocalTaintExprStepConfiguration" }

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

@@ -10,6 +10,7 @@
  */
 
 import csharp
+import semmle.code.csharp.frameworks.system.Collections
 import HashWithoutSalt::PathGraph
 
 /** The C# class `Windows.Security.Cryptography.Core.HashAlgorithmProvider`. */
@@ -93,12 +94,17 @@ predicate hasAnotherHashCall(MethodCall mc) {
 
 /** Holds if a password hash without salt is further processed in another method call. */
 predicate hasFurtherProcessing(MethodCall mc) {
-  mc.getTarget().fromLibrary() and
-  (
-    mc.getTarget().hasFullyQualifiedName("System", "Array", "Copy") or // Array.Copy(passwordHash, 0, password.Length), 0, key, 0, keyLen);
-    mc.getTarget().hasFullyQualifiedName("System", "String", "Concat") or // string.Concat(passwordHash, saltkey)
-    mc.getTarget().hasFullyQualifiedName("System", "Buffer", "BlockCopy") or // Buffer.BlockCopy(passwordHash, 0, allBytes, 0, 20)
-    mc.getTarget().hasFullyQualifiedName("System", "String", "Format") // String.Format("{0}:{1}:{2}", username, salt, password)
+  exists(Method m | m = mc.getTarget() and m.fromLibrary() |
+    m.hasFullyQualifiedName("System", "Array", "Copy") // Array.Copy(passwordHash, 0, password.Length), 0, key, 0, keyLen);
+    or
+    m.hasFullyQualifiedName("System", "String", "Concat") // string.Concat(passwordHash, saltkey)
+    or
+    m.hasFullyQualifiedName("System", "Buffer", "BlockCopy") // Buffer.BlockCopy(passwordHash, 0, allBytes, 0, 20)
+    or
+    m.hasFullyQualifiedName("System", "String", "Format") // String.Format("{0}:{1}:{2}", username, salt, password)
+    or
+    m.getName() = "CopyTo" and
+    m.getDeclaringType().getABaseType*() instanceof SystemCollectionsICollectionInterface // passBytes.CopyTo(rawSalted, 0);
   )
 }
 

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

@@ -116,7 +116,7 @@ void M17()
         {
             var a = new object[] { new object() };
             var b = Reverse(a);
-            Sink(b); // No flow
+            Sink(b); // Flow
             Sink(b[0]); // Flow
         }
 

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

@@ -104,6 +104,7 @@ edges
 | 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 |  |
 | ExternalFlow.cs:117:34:117:49 | { ..., ... } : null [element] : Object | ExternalFlow.cs:117:17:117:17 | access to local variable a : null [element] : Object | provenance |  |
 | ExternalFlow.cs:117:36:117:47 | object creation of type Object : Object | ExternalFlow.cs:117:34:117:49 | { ..., ... } : null [element] : Object | provenance |  |
+| 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 |  |
 | 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 |  |
 | 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 |  |
 | 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
 | ExternalFlow.cs:118:17:118:17 | access to local variable b : null [element] : Object | semmle.label | access to local variable b : null [element] : Object |
 | ExternalFlow.cs:118:21:118:30 | call to method Reverse : null [element] : Object | semmle.label | call to method Reverse : null [element] : Object |
 | ExternalFlow.cs:118:29:118:29 | access to local variable a : null [element] : Object | semmle.label | access to local variable a : null [element] : Object |
+| ExternalFlow.cs:119:18:119:18 | access to local variable b | semmle.label | access to local variable b |
 | ExternalFlow.cs:120:18:120:18 | access to local variable b : null [element] : Object | semmle.label | access to local variable b : null [element] : Object |
 | ExternalFlow.cs:120:18:120:21 | access to array element | semmle.label | access to array element |
 | 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
 | 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 |
 | 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 |
 | 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 |
+| 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 |
 | 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 |
 | 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 |
 | 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

@@ -0,0 +1,13 @@
+public class CollectionTaintTracking
+{
+    public void ImplicitCollectionReadAtSink()
+    {
+        var tainted = Source<object>(1);
+        var arr = new object[] { tainted };
+        Sink(arr); // $ hasTaintFlow=1
+    }
+
+    static T Source<T>(object source) => throw null;
+
+    public static void Sink<T>(T t) { }
+}

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

@@ -0,0 +1,18 @@
+models
+edges
+| 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 |  |
+| 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 |  |
+| 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 |  |
+| CollectionTaintTracking.cs:6:32:6:42 | { ..., ... } : null [element] : Object | CollectionTaintTracking.cs:6:13:6:15 | access to local variable arr : null [element] : Object | provenance |  |
+| CollectionTaintTracking.cs:6:34:6:40 | access to local variable tainted : Object | CollectionTaintTracking.cs:6:32:6:42 | { ..., ... } : null [element] : Object | provenance |  |
+nodes
+| CollectionTaintTracking.cs:5:13:5:19 | access to local variable tainted : Object | semmle.label | access to local variable tainted : Object |
+| CollectionTaintTracking.cs:5:23:5:39 | call to method Source<Object> : Object | semmle.label | call to method Source<Object> : Object |
+| CollectionTaintTracking.cs:6:13:6:15 | access to local variable arr : null [element] : Object | semmle.label | access to local variable arr : null [element] : Object |
+| CollectionTaintTracking.cs:6:32:6:42 | { ..., ... } : null [element] : Object | semmle.label | { ..., ... } : null [element] : Object |
+| CollectionTaintTracking.cs:6:34:6:40 | access to local variable tainted : Object | semmle.label | access to local variable tainted : Object |
+| CollectionTaintTracking.cs:7:14:7:16 | access to local variable arr | semmle.label | access to local variable arr |
+subpaths
+testFailures
+#select
+| 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

@@ -0,0 +1,12 @@
+/**
+ * @kind path-problem
+ */
+
+import csharp
+import utils.test.InlineFlowTest
+import TaintFlowTest<DefaultFlowConfig>
+import PathGraph
+
+from PathNode source, PathNode sink
+where flowPath(source, sink)
+select sink, source, sink, "$@", source, source.toString()

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

@@ -0,0 +1,2 @@
+semmle-extractor-options: /nostdlib /noconfig
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj