Skip to content

Commit 0e866a5

Browse files
authored
Merge pull request github#15359 from egregius313/egregius313/csharp/dataflow/threat-modeling/add-threatmodelflowsource
C#: Threat Modeling - Introduce `ThreatModelFlowSource`
2 parents df8d453 + a6c977c commit 0e866a5

30 files changed

+530
-11
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 a new library `semmle.code.csharp.security.dataflow.flowsources.FlowSources`, which provides a new class `ThreatModelFlowSource`. The `ThreatModelFlowSource` class can be used to include sources which match the current *threat model* configuration.

csharp/ql/lib/qlpack.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ dependencies:
1010
codeql/dataflow: ${workspace}
1111
codeql/mad: ${workspace}
1212
codeql/ssa: ${workspace}
13+
codeql/threat-models: ${workspace}
1314
codeql/tutorial: ${workspace}
1415
codeql/util: ${workspace}
1516
dataExtensions:

csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ private import semmle.code.csharp.frameworks.Sql
1212
private import semmle.code.csharp.dataflow.internal.FlowSummaryImpl::Public
1313
private import semmle.code.csharp.dataflow.internal.FlowSummaryImpl::Private
1414
private import semmle.code.csharp.dataflow.internal.DataFlowPrivate as DataFlowPrivate
15+
private import semmle.code.csharp.security.dataflow.flowsources.Stored as Stored
1516

1617
/**
1718
* Definitions relating to the `System.ComponentModel.DataAnnotations`
@@ -44,7 +45,7 @@ module EntityFramework {
4445
}
4546

4647
/** A taint source where the data has come from a mapped property stored in the database. */
47-
class StoredFlowSource extends DataFlow::Node {
48+
class StoredFlowSource extends Stored::DatabaseInputSource {
4849
StoredFlowSource() {
4950
this.asExpr() = any(PropertyRead read | read.getTarget() instanceof MappedProperty)
5051
}

csharp/ql/lib/semmle/code/csharp/frameworks/NHibernate.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import csharp
66
private import semmle.code.csharp.frameworks.System
77
private import semmle.code.csharp.frameworks.system.Collections
88
private import semmle.code.csharp.frameworks.Sql
9+
private import semmle.code.csharp.security.dataflow.flowsources.Stored as Stored
910

1011
/** Definitions relating to the `NHibernate` package. */
1112
module NHibernate {
@@ -86,7 +87,7 @@ module NHibernate {
8687
}
8788

8889
/** A taint source where the data has come from a mapped property stored in the database. */
89-
class StoredFlowSource extends DataFlow::Node {
90+
class StoredFlowSource extends Stored::DatabaseInputSource {
9091
StoredFlowSource() {
9192
this.asExpr() = any(PropertyRead read | read.getTarget() instanceof MappedProperty)
9293
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/** Provides classes representing various flow sources for taint tracking. */
2+
3+
private import semmle.code.csharp.dataflow.internal.ExternalFlow
4+
private import codeql.threatmodels.ThreatModels
5+
import semmle.code.csharp.security.dataflow.flowsources.Remote
6+
import semmle.code.csharp.security.dataflow.flowsources.Local
7+
import semmle.code.csharp.security.dataflow.flowsources.Stored
8+
9+
/**
10+
* A data flow source.
11+
*/
12+
abstract class SourceNode extends DataFlow::Node {
13+
/**
14+
* Gets a string that represents the source kind with respect to threat modeling.
15+
*/
16+
abstract string getThreatModel();
17+
}
18+
19+
/**
20+
* A class of data flow sources that respects the
21+
* current threat model configuration.
22+
*/
23+
class ThreatModelFlowSource extends DataFlow::Node {
24+
ThreatModelFlowSource() {
25+
exists(string kind |
26+
// Specific threat model.
27+
currentThreatModel(kind) and
28+
(this.(SourceNode).getThreatModel() = kind or sourceNode(this, kind))
29+
)
30+
}
31+
}

csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Local.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@
55
import csharp
66
private import semmle.code.csharp.frameworks.system.windows.Forms
77
private import semmle.code.csharp.dataflow.internal.ExternalFlow
8+
private import semmle.code.csharp.security.dataflow.flowsources.FlowSources
89

910
/** A data flow source of local data. */
10-
abstract class LocalFlowSource extends DataFlow::Node {
11+
abstract class LocalFlowSource extends SourceNode {
1112
/** Gets a string that describes the type of this local flow source. */
1213
abstract string getSourceType();
14+
15+
override string getThreatModel() { result = "local" }
1316
}
1417

1518
private class ExternalLocalFlowSource extends LocalFlowSource {

csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@ private import semmle.code.csharp.frameworks.WCF
1313
private import semmle.code.csharp.frameworks.microsoft.Owin
1414
private import semmle.code.csharp.frameworks.microsoft.AspNetCore
1515
private import semmle.code.csharp.dataflow.internal.ExternalFlow
16+
private import semmle.code.csharp.security.dataflow.flowsources.FlowSources
1617

1718
/** A data flow source of remote user input. */
18-
abstract class RemoteFlowSource extends DataFlow::Node {
19+
abstract class RemoteFlowSource extends SourceNode {
1920
/** Gets a string that describes the type of this remote flow source. */
2021
abstract string getSourceType();
22+
23+
override string getThreatModel() { result = "remote" }
2124
}
2225

2326
/**

csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Stored.qll

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,25 @@ private import semmle.code.csharp.frameworks.system.data.Entity
99
private import semmle.code.csharp.frameworks.EntityFramework
1010
private import semmle.code.csharp.frameworks.NHibernate
1111
private import semmle.code.csharp.frameworks.Sql
12+
private import semmle.code.csharp.security.dataflow.flowsources.FlowSources
1213

1314
/** A data flow source of stored user input. */
14-
abstract class StoredFlowSource extends DataFlow::Node { }
15+
abstract class StoredFlowSource extends SourceNode {
16+
override string getThreatModel() { result = "local" }
17+
}
18+
19+
/**
20+
* A node with input from a database.
21+
*/
22+
abstract class DatabaseInputSource extends StoredFlowSource {
23+
override string getThreatModel() { result = "database" }
24+
}
1525

1626
/**
1727
* An expression that has a type of `DbRawSqlQuery`, representing the result of an Entity Framework
1828
* SqlQuery.
1929
*/
20-
class DbRawSqlStoredFlowSource extends StoredFlowSource {
30+
class DbRawSqlStoredFlowSource extends DatabaseInputSource {
2131
DbRawSqlStoredFlowSource() {
2232
this.asExpr().getType() instanceof SystemDataEntityInfrastructure::DbRawSqlQuery
2333
}
@@ -27,30 +37,34 @@ class DbRawSqlStoredFlowSource extends StoredFlowSource {
2737
* An expression that has a type of `DbDataReader` or a sub-class, representing the result of a
2838
* data command.
2939
*/
30-
class DbDataReaderStoredFlowSource extends StoredFlowSource {
40+
class DbDataReaderStoredFlowSource extends DatabaseInputSource {
3141
DbDataReaderStoredFlowSource() {
3242
this.asExpr().getType() = any(SystemDataCommon::DbDataReader dataReader).getASubType*()
3343
}
3444
}
3545

3646
/** An expression that accesses a method of `DbDataReader` or a sub-class. */
37-
class DbDataReaderMethodStoredFlowSource extends StoredFlowSource {
47+
class DbDataReaderMethodStoredFlowSource extends DatabaseInputSource {
3848
DbDataReaderMethodStoredFlowSource() {
3949
this.asExpr().(MethodCall).getTarget().getDeclaringType() =
4050
any(SystemDataCommon::DbDataReader dataReader).getASubType*()
4151
}
4252
}
4353

4454
/** An expression that accesses a property of `DbDataReader` or a sub-class. */
45-
class DbDataReaderPropertyStoredFlowSource extends StoredFlowSource {
55+
class DbDataReaderPropertyStoredFlowSource extends DatabaseInputSource {
4656
DbDataReaderPropertyStoredFlowSource() {
4757
this.asExpr().(PropertyAccess).getTarget().getDeclaringType() =
4858
any(SystemDataCommon::DbDataReader dataReader).getASubType*()
4959
}
5060
}
5161

52-
/** A read of a mapped property. */
53-
class ORMMappedProperty extends StoredFlowSource {
62+
/**
63+
* DEPRECATED: Use `EntityFramework::StoredFlowSource` and `NHibernate::StoredFlowSource` instead.
64+
*
65+
* A read of a mapped property.
66+
*/
67+
deprecated class ORMMappedProperty extends DataFlow::Node {
5468
ORMMappedProperty() {
5569
this instanceof EntityFramework::StoredFlowSource or
5670
this instanceof NHibernate::StoredFlowSource
@@ -60,4 +74,6 @@ class ORMMappedProperty extends StoredFlowSource {
6074
/** A file stream source is considered a stored flow source. */
6175
class FileStreamStoredFlowSource extends StoredFlowSource {
6276
FileStreamStoredFlowSource() { sourceNode(this, "file") }
77+
78+
override string getThreatModel() { result = "file" }
6379
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
using System.Net.Sockets;
2+
using System.Data.SqlClient;
3+
4+
namespace My.Qltest
5+
{
6+
public class Test
7+
{
8+
private TestSources Sources = new TestSources();
9+
10+
private SqlConnection Connection => throw null;
11+
12+
private string BytesToString(byte[] bytes)
13+
{
14+
// Encode bytes to a UTF8 string.
15+
return System.Text.Encoding.UTF8.GetString(bytes);
16+
}
17+
18+
public void M1()
19+
{
20+
// Only a source if "remote" is a selected threat model.
21+
// This is included in the "default" threat model.
22+
using TcpClient client = new TcpClient("localhost", 1234);
23+
using NetworkStream stream = client.GetStream();
24+
byte[] buffer = new byte[1024];
25+
int bytesRead = stream.Read(buffer, 0, buffer.Length);
26+
27+
// SQL sink
28+
var command = new SqlCommand("SELECT * FROM Users WHERE Username = '" + BytesToString(buffer) + "'", Connection);
29+
}
30+
31+
public void M2()
32+
{
33+
// Only a source if "database" is a selected threat model.
34+
string result = Sources.ExecuteQuery("SELECT * FROM foo");
35+
36+
// SQL sink
37+
var command = new SqlCommand("SELECT * FROM Users WHERE Username = '" + result + "'", Connection);
38+
}
39+
40+
public void M3()
41+
{
42+
// Only a source if "environment" is a selected threat model.
43+
string result = Sources.ReadEnv("foo");
44+
45+
// SQL sink
46+
var command = new SqlCommand("SELECT * FROM Users WHERE Username = '" + result + "'", Connection);
47+
48+
}
49+
50+
public void M4()
51+
{
52+
// Only a source if "custom" is a selected threat model.
53+
string result = Sources.GetCustom("foo");
54+
55+
// SQL sink
56+
var command = new SqlCommand("SELECT * FROM Users WHERE Username = '" + result + "'", Connection);
57+
}
58+
59+
public void M5()
60+
{
61+
// Only a source if "commandargs" is a selected threat model.
62+
string result = Sources.GetCliArg(0);
63+
64+
// SQL sink
65+
var command = new SqlCommand("SELECT * FROM Users WHERE Username = '" + result + "'", Connection);
66+
}
67+
}
68+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
private import csharp
2+
private import semmle.code.csharp.dataflow.DataFlow
3+
private import semmle.code.csharp.dataflow.internal.ExternalFlow
4+
private import semmle.code.csharp.security.dataflow.flowsources.FlowSources
5+
6+
private module ThreatModelConfig implements DataFlow::ConfigSig {
7+
predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
8+
9+
predicate isSink(DataFlow::Node sink) { sinkNode(sink, _) }
10+
}
11+
12+
module ThreatModel = TaintTracking::Global<ThreatModelConfig>;

0 commit comments

Comments
 (0)