hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 02:05:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #20089 from michaelnebel/csharp/allowsinkimplicitread

Browse Source 
C#: Allow implicit collection reads in sink nodes.
This commit is contained in:
Michael Nebel
2025-08-21 15:29:52 +02:00
committed by GitHub
parent 771d7cb171 ebfbc71104
commit c89f2e309d
9 changed files with 69 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
csharp/ql/lib/change-notes/2025-08-18-implicit-reads-at-sinks.md Normal file
View File

@@ -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.

5
csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll
View File

@@ -29,7 +29,10 @@ 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) {
exists(node) and
c.isElement()
}
private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityConfiguration {
LocalTaintExprStepConfiguration() { this = "LocalTaintExprStepConfiguration" }

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

@@ -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);
)
}

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

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

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

@@ -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 |

13
csharp/ql/test/library-tests/tainttracking/collections/CollectionTaintTracking.cs Normal file
View File

@@ -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) { }
}

18
csharp/ql/test/library-tests/tainttracking/collections/CollectionTaintTracking.expected Normal file
View File

@@ -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 |

12
csharp/ql/test/library-tests/tainttracking/collections/CollectionTaintTracking.ql Normal file
View File

@@ -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()

2
csharp/ql/test/library-tests/tainttracking/collections/options Normal file
View File

@@ -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