Skip to content

Commit e7852f5

Browse files
egregius313egregius313
authored
Merge pull request #15605 from egregius313/egregius313/csharp/dataflow/sources/commandargs-and-environment
C#: Add more `environment` and `commandargs` sources for the C# Standard Library
2 parents 5440dbf + 7f950d8 commit e7852f5

22 files changed

+254
-18
lines changed

‎csharp/ql/lib/change-notes/2024-03-05-new-commandargs-and-environment-models.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The .NET standard libraries APIs for accessing command line arguments and environment variables have been modeled using the `commandargs` and `environment` threat models.
5+
* The `cs/assembly-path-injection` query has been modified so that it's sources rely on `ThreatModelFlowSource`. In order to restore results from command line arguments, you should enable the `commandargs` threat model.

‎csharp/ql/lib/ext/Microsoft.Extensions.Configuration.model.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/csharp-all
4+
extensible: sourceModel
5+
data:
6+
- ["Microsoft.Extensions.Configuration", "EnvironmentVariablesExtensions", False, "AddEnvironmentVariables", "", "", "Argument[0]", "environment", "manual"]
7+
- ["Microsoft.Extensions.Configuration", "EnvironmentVariablesExtensions", False, "AddEnvironmentVariables", "", "", "ReturnValue", "environment", "manual"]
8+
- addsTo:
9+
pack: codeql/csharp-all
10+
extensible: summaryModel
11+
data:
12+
- ["Microsoft.Extensions.Configuration", "IConfiguration", True, "get_Item", "(System.String)", "", "Argument[this]", "ReturnValue", "taint", "manual"]
13+
- ["Microsoft.Extensions.Configuration", "IConfigurationBuilder", True, "Build", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"]
14+
- ["Microsoft.Extensions.Configuration", "CommandLineConfigurationExtensions", False, "AddCommandLine", "", "", "Argument[1]", "Argument[0]", "taint", "manual"]
15+
- ["Microsoft.Extensions.Configuration", "CommandLineConfigurationExtensions", False, "AddCommandLine", "(Microsoft.Extensions.Configuration.IConfigurationBuilder,System.String[],System.Collections.Generic.IDictionary<System.String,System.String>)", "", "Argument[2]", "Argument[0]", "taint", "manual"]
16+
- ["Microsoft.Extensions.Configuration", "CommandLineConfigurationExtensions", False, "AddCommandLine", "", "", "Argument[1]", "ReturnValue", "taint", "manual"]
17+
- ["Microsoft.Extensions.Configuration", "CommandLineConfigurationExtensions", False, "AddCommandLine", "(Microsoft.Extensions.Configuration.IConfigurationBuilder,System.String[],System.Collections.Generic.IDictionary<System.String,System.String>)", "", "Argument[2]", "ReturnValue", "taint", "manual"]

‎csharp/ql/lib/ext/System.model.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ extensions:
66
- ["System", "Console", False, "Read", "", "", "ReturnValue", "local", "manual"]
77
- ["System", "Console", False, "ReadKey", "", "", "ReturnValue", "local", "manual"]
88
- ["System", "Console", False, "ReadLine", "", "", "ReturnValue", "local", "manual"]
9+
- ["System", "Environment", False, "ExpandEnvironmentVariables", "(System.String)", "", "ReturnValue", "environment", "manual"]
10+
- ["System", "Environment", False, "GetCommandLineArgs", "()", "", "ReturnValue", "commandargs", "manual"]
11+
- ["System", "Environment", False, "get_CommandLine", "()", "", "ReturnValue", "commandargs", "manual"]
12+
- ["System", "Environment", False, "GetEnvironmentVariable", "", "", "ReturnValue", "environment", "manual"]
13+
- ["System", "Environment", False, "GetEnvironmentVariables", "", "", "ReturnValue", "environment", "manual"]
914
- addsTo:
1015
pack: codeql/csharp-all
1116
extensible: summaryModel

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import csharp
66
private import semmle.code.csharp.frameworks.system.windows.Forms
77
private import semmle.code.csharp.dataflow.internal.ExternalFlow
88
private import semmle.code.csharp.security.dataflow.flowsources.FlowSources
9+
private import semmle.code.csharp.commons.Util
910

1011
/** A data flow source of local data. */
1112
abstract class LocalFlowSource extends SourceNode {
@@ -29,3 +30,28 @@ class TextFieldSource extends LocalUserInputSource {
2930

3031
override string getSourceType() { result = "TextBox text" }
3132
}
33+
34+
/**
35+
* A dataflow source that represents the access of an environment variable.
36+
*/
37+
abstract class EnvironmentVariableSource extends LocalFlowSource {
38+
override string getThreatModel() { result = "environment" }
39+
40+
override string getSourceType() { result = "environment variable" }
41+
}
42+
43+
/**
44+
* A dataflow source that represents the access of a command line argument.
45+
*/
46+
abstract class CommandLineArgumentSource extends LocalFlowSource {
47+
override string getThreatModel() { result = "commandargs" }
48+
49+
override string getSourceType() { result = "command line argument" }
50+
}
51+
52+
/**
53+
* A data flow source that represents the parameters of the `Main` method of a program.
54+
*/
55+
private class MainMethodArgumentSource extends CommandLineArgumentSource {
56+
MainMethodArgumentSource() { this.asParameter() = any(MainMethod mainMethod).getAParameter() }
57+
}

‎csharp/ql/src/Security Features/CWE-114/AssemblyPathInjection.ql

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,7 @@ import AssemblyPathInjection::PathGraph
2121
* A taint-tracking configuration for untrusted user input used to load a DLL.
2222
*/
2323
module AssemblyPathInjectionConfig implements DataFlow::ConfigSig {
24-
predicate isSource(DataFlow::Node source) {
25-
source instanceof ThreatModelFlowSource or
26-
source.asExpr() = any(MainMethod main).getParameter(0).getAnAccess()
27-
}
24+
predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
2825

2926
predicate isSink(DataFlow::Node sink) {
3027
exists(MethodCall mc, string name, int arg |

‎csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/CommandArgs.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using Microsoft.Extensions.Configuration;
4+
5+
namespace CommandArgs
6+
{
7+
public class CommandArgsUse
8+
{
9+
public static void M1()
10+
{
11+
string result = Environment.GetCommandLineArgs()[0];
12+
}
13+
14+
public static void M2()
15+
{
16+
string result = Environment.CommandLine;
17+
}
18+
19+
public static void Main(string[] args)
20+
{
21+
var builder = new ConfigurationBuilder();
22+
builder.AddCommandLine(args);
23+
var config = builder.Build();
24+
var arg1 = config["arg1"];
25+
Sink(arg1);
26+
}
27+
28+
public static void AddCommandLine2()
29+
{
30+
var config = new ConfigurationBuilder()
31+
.AddCommandLine(Environment.GetCommandLineArgs())
32+
.Build();
33+
var arg1 = config["arg1"];
34+
Sink(arg1);
35+
}
36+
37+
static void Sink(object o) { }
38+
}
39+
}

‎csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/CommandArgs.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| CommandArgs.cs:11:29:11:60 | call to method GetCommandLineArgs |
2+
| CommandArgs.cs:16:29:16:51 | access to property CommandLine |
3+
| CommandArgs.cs:19:42:19:45 | args |
4+
| CommandArgs.cs:31:33:31:64 | call to method GetCommandLineArgs |

‎csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/CommandArgs.ext.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extensions:
2+
3+
- addsTo:
4+
pack: codeql/threat-models
5+
extensible: threatModelConfiguration
6+
data:
7+
- ["commandargs", true, 0]

‎csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/CommandArgs.ql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import csharp
2+
import semmle.code.csharp.security.dataflow.flowsources.FlowSources
3+
4+
from DataFlow::Node source
5+
where source instanceof ThreatModelFlowSource
6+
select source

‎csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/CommandLineFlow.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| CommandArgs.cs:25:18:25:21 | access to local variable arg1 | CommandArgs.cs:19:42:19:45 | args |
2+
| CommandArgs.cs:34:18:34:21 | access to local variable arg1 | CommandArgs.cs:31:33:31:64 | call to method GetCommandLineArgs |

‎csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/CommandLineFlow.ext.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extensions:
2+
3+
- addsTo:
4+
pack: codeql/threat-models
5+
extensible: threatModelConfiguration
6+
data:
7+
- ["commandargs", true, 0]

‎csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/CommandLineFlow.ql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import csharp
2+
import semmle.code.csharp.security.dataflow.flowsources.FlowSources
3+
4+
module CommandLineFlowConfig implements DataFlow::ConfigSig {
5+
predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
6+
7+
predicate isSink(DataFlow::Node sink) {
8+
exists(MethodCall mc | mc.getTarget().hasName("Sink") | sink.asExpr() = mc.getArgument(0))
9+
}
10+
}
11+
12+
module CommandLineFlow = TaintTracking::Global<CommandLineFlowConfig>;
13+
14+
from DataFlow::Node source, DataFlow::Node sink
15+
where CommandLineFlow::flow(source, sink)
16+
select sink, source

‎csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/options

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

‎csharp/ql/test/library-tests/dataflow/flowsources/local/environment/EnvironmentVariableFlow.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| EnvironmentVariables.cs:34:18:34:21 | access to local variable path | EnvironmentVariables.cs:30:26:31:42 | call to method AddEnvironmentVariables |
2+
| EnvironmentVariables.cs:44:18:44:21 | access to local variable path | EnvironmentVariables.cs:41:13:41:19 | [post] access to local variable builder |

‎csharp/ql/test/library-tests/dataflow/flowsources/local/environment/EnvironmentVariableFlow.ql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import csharp
2+
import semmle.code.csharp.dataflow.internal.ExternalFlow
3+
4+
module EnvironmentVariableFlowConfig implements DataFlow::ConfigSig {
5+
predicate isSource(DataFlow::Node source) { sourceNode(source, "environment") }
6+
7+
predicate isSink(DataFlow::Node sink) {
8+
exists(MethodCall mc | mc.getTarget().hasName("Sink") | sink.asExpr() = mc.getArgument(0))
9+
}
10+
}
11+
12+
module EnvironmentVariableFlow = TaintTracking::Global<EnvironmentVariableFlowConfig>;
13+
14+
from DataFlow::Node source, DataFlow::Node sink
15+
where EnvironmentVariableFlow::flow(source, sink)
16+
select sink, source

‎csharp/ql/test/library-tests/dataflow/flowsources/local/environment/EnvironmentVariables.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
using System;
2+
using System.Collections;
3+
using Microsoft.Extensions.Configuration;
4+
5+
namespace EnvironmentVariables
6+
{
7+
class EnvironmentVariables
8+
{
9+
public static void GetEnvironmentVariable(string environmnetVariable)
10+
{
11+
string value = Environment.GetEnvironmentVariable(environmnetVariable);
12+
string valueFromRegistry = Environment.GetEnvironmentVariable(environmnetVariable, EnvironmentVariableTarget.Machine);
13+
string valueFromProcess = Environment.GetEnvironmentVariable(environmnetVariable, EnvironmentVariableTarget.Process);
14+
}
15+
16+
public static void GetEnvironmentVariables()
17+
{
18+
IDictionary environmentVariables = Environment.GetEnvironmentVariables();
19+
IDictionary environmentVariablesFromRegistry = Environment.GetEnvironmentVariables(EnvironmentVariableTarget.Machine);
20+
IDictionary environmentVariablesFromProcess = Environment.GetEnvironmentVariables(EnvironmentVariableTarget.Process);
21+
}
22+
23+
public static void ExpandEnvironmentVariables(string environmentVariable)
24+
{
25+
string expanded = Environment.ExpandEnvironmentVariables("%PATH%");
26+
}
27+
28+
public static void TaintedConfiguration()
29+
{
30+
var config = new ConfigurationBuilder()
31+
.AddEnvironmentVariables()
32+
.Build();
33+
var path = config["PATH"];
34+
Sink(path);
35+
}
36+
37+
public static void TaintedConfigurationWithPrefix()
38+
{
39+
var builder = new ConfigurationBuilder();
40+
41+
builder.AddEnvironmentVariables("prefix");
42+
var config = builder.Build();
43+
var path = config["PATH"];
44+
Sink(path);
45+
}
46+
47+
static void Sink(object o) { }
48+
}
49+
}

‎csharp/ql/test/library-tests/dataflow/flowsources/local/environment/EnvironmentVariables.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
| EnvironmentVariables.cs:11:28:11:82 | call to method GetEnvironmentVariable |
2+
| EnvironmentVariables.cs:12:40:12:129 | call to method GetEnvironmentVariable |
3+
| EnvironmentVariables.cs:13:39:13:128 | call to method GetEnvironmentVariable |
4+
| EnvironmentVariables.cs:18:48:18:84 | call to method GetEnvironmentVariables |
5+
| EnvironmentVariables.cs:19:60:19:129 | call to method GetEnvironmentVariables |
6+
| EnvironmentVariables.cs:20:59:20:128 | call to method GetEnvironmentVariables |
7+
| EnvironmentVariables.cs:25:31:25:78 | call to method ExpandEnvironmentVariables |
8+
| EnvironmentVariables.cs:30:26:30:51 | [post] object creation of type ConfigurationBuilder |
9+
| EnvironmentVariables.cs:30:26:31:42 | call to method AddEnvironmentVariables |
10+
| EnvironmentVariables.cs:41:13:41:19 | [post] access to local variable builder |
11+
| EnvironmentVariables.cs:41:13:41:53 | call to method AddEnvironmentVariables |

‎csharp/ql/test/library-tests/dataflow/flowsources/local/environment/EnvironmentVariables.ql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import csharp
2+
import semmle.code.csharp.dataflow.internal.ExternalFlow
3+
4+
from DataFlow::Node source
5+
where sourceNode(source, "environment")
6+
select source

‎csharp/ql/test/library-tests/dataflow/flowsources/local/environment/options

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

0 commit comments

Comments
 (0)