C#: Add more `environment` and `commandargs` sources for the C# Standard Library #15605

egregius313 · 2024-02-13T15:44:52Z

Adds more models for the environment and commandargs local source kinds.

This primarily focuses on the .NET standard library and the Microsoft.Extensions.Configuration library.

github-actions · 2024-02-13T15:48:16Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

Changes to framework-coverage-csharp.rst:

-    System,"``System.*``, ``System``",25,11862,67,9
+    System,"``System.*``, ``System``",30,11862,67,9
-    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32.SafeHandles``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",6,1541,148,
+    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32.SafeHandles``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",8,1547,148,
-    Totals,,31,13410,409,9
+    Totals,,38,13416,409,9

Changes to framework-coverage-csharp.csv:

- package,sink,source,summary,sink:code-injection,sink:encryption-decryptor,sink:encryption-encryptor,sink:encryption-keyprop,sink:encryption-symmetrickey,sink:file-content-store,sink:html-injection,sink:js-injection,sink:log-injection,sink:sql-injection,source:file,source:file-write,source:local,source:remote,summary:taint,summary:value
+ package,sink,source,summary,sink:code-injection,sink:encryption-decryptor,sink:encryption-encryptor,sink:encryption-keyprop,sink:encryption-symmetrickey,sink:file-content-store,sink:html-injection,sink:js-injection,sink:log-injection,sink:sql-injection,source:commandargs,source:environment,source:file,source:file-write,source:local,source:remote,summary:taint,summary:value
- Amazon.Lambda.APIGatewayEvents,,6,,,,,,,,,,,,,,,6,,
+ Amazon.Lambda.APIGatewayEvents,,6,,,,,,,,,,,,,,,,,6,,
- Amazon.Lambda.Core,10,,,,,,,,,,,10,,,,,,,
+ Amazon.Lambda.Core,10,,,,,,,,,,,10,,,,,,,,,
- Dapper,55,,,,,,,,,,,,55,,,,,,
+ Dapper,55,,,,,,,,,,,,55,,,,,,,,
- ILCompiler,,,81,,,,,,,,,,,,,,,81,
+ ILCompiler,,,81,,,,,,,,,,,,,,,,,81,
- ILLink.RoslynAnalyzer,,,63,,,,,,,,,,,,,,,63,
+ ILLink.RoslynAnalyzer,,,63,,,,,,,,,,,,,,,,,63,
- ILLink.Shared,,,32,,,,,,,,,,,,,,,29,3
+ ILLink.Shared,,,32,,,,,,,,,,,,,,,,,29,3
- ILLink.Tasks,,,5,,,,,,,,,,,,,,,5,
+ ILLink.Tasks,,,5,,,,,,,,,,,,,,,,,5,
- Internal.IL,,,69,,,,,,,,,,,,,,,67,2
+ Internal.IL,,,69,,,,,,,,,,,,,,,,,67,2
- Internal.Pgo,,,9,,,,,,,,,,,,,,,8,1
+ Internal.Pgo,,,9,,,,,,,,,,,,,,,,,8,1
- Internal.TypeSystem,,,367,,,,,,,,,,,,,,,331,36
+ Internal.TypeSystem,,,367,,,,,,,,,,,,,,,,,331,36
- JsonToItemsTaskFactory,,,7,,,,,,,,,,,,,,,7,
+ JsonToItemsTaskFactory,,,7,,,,,,,,,,,,,,,,,7,
- Microsoft.Android.Build,,,14,,,,,,,,,,,,,,,14,
+ Microsoft.Android.Build,,,14,,,,,,,,,,,,,,,,,14,
- Microsoft.Apple.Build,,,7,,,,,,,,,,,,,,,7,
+ Microsoft.Apple.Build,,,7,,,,,,,,,,,,,,,,,7,
- Microsoft.ApplicationBlocks.Data,28,,,,,,,,,,,,28,,,,,,
+ Microsoft.ApplicationBlocks.Data,28,,,,,,,,,,,,28,,,,,,,,
- Microsoft.CSharp,,,24,,,,,,,,,,,,,,,24,
+ Microsoft.CSharp,,,24,,,,,,,,,,,,,,,,,24,
- Microsoft.Diagnostics.Tools.Pgo,,,13,,,,,,,,,,,,,,,13,
+ Microsoft.Diagnostics.Tools.Pgo,,,13,,,,,,,,,,,,,,,,,13,
- Microsoft.EntityFrameworkCore,6,,12,,,,,,,,,,6,,,,,,12
+ Microsoft.EntityFrameworkCore,6,,12,,,,,,,,,,6,,,,,,,,12
- Microsoft.Extensions.Caching.Distributed,,,15,,,,,,,,,,,,,,,15,
+ Microsoft.Extensions.Caching.Distributed,,,15,,,,,,,,,,,,,,,,,15,
- Microsoft.Extensions.Caching.Memory,,,38,,,,,,,,,,,,,,,37,1
+ Microsoft.Extensions.Caching.Memory,,,38,,,,,,,,,,,,,,,,,37,1
- Microsoft.Extensions.Configuration,,,83,,,,,,,,,,,,,,,80,3
+ Microsoft.Extensions.Configuration,,2,89,,,,,,,,,,,,2,,,,,86,3
- Microsoft.Extensions.DependencyInjection,,,120,,,,,,,,,,,,,,,120,
+ Microsoft.Extensions.DependencyInjection,,,120,,,,,,,,,,,,,,,,,120,
- Microsoft.Extensions.DependencyModel,,,12,,,,,,,,,,,,,,,12,
+ Microsoft.Extensions.DependencyModel,,,12,,,,,,,,,,,,,,,,,12,
- Microsoft.Extensions.Diagnostics.Metrics,,,13,,,,,,,,,,,,,,,13,
+ Microsoft.Extensions.Diagnostics.Metrics,,,13,,,,,,,,,,,,,,,,,13,
- Microsoft.Extensions.FileProviders,,,15,,,,,,,,,,,,,,,15,
+ Microsoft.Extensions.FileProviders,,,15,,,,,,,,,,,,,,,,,15,
- Microsoft.Extensions.FileSystemGlobbing,,,16,,,,,,,,,,,,,,,14,2
+ Microsoft.Extensions.FileSystemGlobbing,,,16,,,,,,,,,,,,,,,,,14,2
- Microsoft.Extensions.Hosting,,,23,,,,,,,,,,,,,,,22,1
+ Microsoft.Extensions.Hosting,,,23,,,,,,,,,,,,,,,,,22,1
- Microsoft.Extensions.Http,,,10,,,,,,,,,,,,,,,10,
+ Microsoft.Extensions.Http,,,10,,,,,,,,,,,,,,,,,10,
- Microsoft.Extensions.Logging,,,60,,,,,,,,,,,,,,,59,1
+ Microsoft.Extensions.Logging,,,60,,,,,,,,,,,,,,,,,59,1
- Microsoft.Extensions.Options,,,8,,,,,,,,,,,,,,,8,
+ Microsoft.Extensions.Options,,,8,,,,,,,,,,,,,,,,,8,
- Microsoft.Extensions.Primitives,,,64,,,,,,,,,,,,,,,64,
+ Microsoft.Extensions.Primitives,,,64,,,,,,,,,,,,,,,,,64,
- Microsoft.Interop,,,78,,,,,,,,,,,,,,,78,
+ Microsoft.Interop,,,78,,,,,,,,,,,,,,,,,78,
- Microsoft.NET.Build.Tasks,,,1,,,,,,,,,,,,,,,1,
+ Microsoft.NET.Build.Tasks,,,1,,,,,,,,,,,,,,,,,1,
- Microsoft.NET.WebAssembly.Webcil,,,7,,,,,,,,,,,,,,,7,
+ Microsoft.NET.WebAssembly.Webcil,,,7,,,,,,,,,,,,,,,,,7,
- Microsoft.VisualBasic,,,10,,,,,,,,,,,,,,,5,5
+ Microsoft.VisualBasic,,,10,,,,,,,,,,,,,,,,,5,5
- Microsoft.WebAssembly.Build.Tasks,,,3,,,,,,,,,,,,,,,3,
+ Microsoft.WebAssembly.Build.Tasks,,,3,,,,,,,,,,,,,,,,,3,
- Microsoft.Win32.SafeHandles,,,4,,,,,,,,,,,,,,,4,
+ Microsoft.Win32.SafeHandles,,,4,,,,,,,,,,,,,,,,,4,
- Mono.Linker,,,163,,,,,,,,,,,,,,,163,
+ Mono.Linker,,,163,,,,,,,,,,,,,,,,,163,
- MySql.Data.MySqlClient,48,,,,,,,,,,,,48,,,,,,
+ MySql.Data.MySqlClient,48,,,,,,,,,,,,48,,,,,,,,
- Newtonsoft.Json,,,91,,,,,,,,,,,,,,,73,18
+ Newtonsoft.Json,,,91,,,,,,,,,,,,,,,,,73,18
- ServiceStack,194,,7,27,,,,,75,,,,92,,,,,7,
+ ServiceStack,194,,7,27,,,,,75,,,,92,,,,,,,7,
- SourceGenerators,,,4,,,,,,,,,,,,,,,4,
+ SourceGenerators,,,4,,,,,,,,,,,,,,,,,4,
- System,67,25,11862,,8,8,9,,,4,5,,33,1,17,3,4,9896,1966
+ System,67,30,11862,,8,8,9,,,4,5,,33,2,3,1,17,3,4,9896,1966
- Windows.Security.Cryptography.Core,1,,,,,,,1,,,,,,,,,,,
+ Windows.Security.Cryptography.Core,1,,,,,,,1,,,,,,,,,,,,,

michaelnebel

Thank you!
I have added some comments.
Could you elaborate a bit on, how you found the sources?

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

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

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

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

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

...sources/stubs/Microsoft.Extensions.Configuration/8.0.0/Microsoft.Extensions.Configuration.cs

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

egregius313 · 2024-03-06T18:12:28Z

Could you elaborate a bit on, how you found the sources?

I was looking into packages under the System.* and Microsoft.* namespaces (.NET standard library and Microsoft libraries) for APIs dealing with environment variables and/or command line arguments. And in the process, I found System.CommandLine and Microsoft.Extensions.Configuration.

But I've currently paused the System.CommandLine modeling for now, because it mostly involved marking parameters to callbacks as sources, which is currently unsupported in MaD for C# as I understand it.

The standard library models were mostly from looking into the standard way to access command line arguments and environment variables with System.Environment.

egregius313 · 2024-03-07T05:29:21Z

@michaelnebel I have modified the Microsoft.Extensions.Configuration models to handle the fluent API, and have added taint flow tests for the extension methods to check that things are being tracked appropriately with both versions of the fluent API.

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

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

michaelnebel · 2024-03-08T09:38:50Z

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

+import semmle.code.csharp.dataflow.internal.ExternalFlow
+
+module EnvironmentVariableFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { sourceNode(source, "environment") }


nit: You can consider to do this using threat models (also fine to leave it as it is).

michaelnebel

Excellent!
I will start a DCA run; Also we need to hold the merge of this PR as one of the queries now relies on threat models.

michaelnebel · 2024-03-08T09:41:31Z

Excellent! I will start a DCA run; Also we need to hold the merge of this PR as one of the queries now relies on threat models.

Alternatively split the PR in two such that it is follow up PR that changes the semantics of the query.

michaelnebel · 2024-03-08T09:45:36Z

Excellent! I will start a DCA run; Also we need to hold the merge of this PR as one of the queries now relies on threat models.

Alternatively split the PR in two such that it is follow up PR that changes the semantics of the query.

Looks like there is no need for this.
I think that we can start to merge all the threat model PRs.

github-actions bot added the C# label Feb 13, 2024

egregius313 force-pushed the egregius313/csharp/dataflow/sources/commandargs-and-environment branch 2 times, most recently from ceb169b to b32a08a Compare February 26, 2024 17:58

egregius313 force-pushed the egregius313/csharp/dataflow/sources/commandargs-and-environment branch 3 times, most recently from 09f00b3 to 386cf4d Compare March 6, 2024 03:28

egregius313 marked this pull request as ready for review March 6, 2024 03:28

egregius313 requested a review from a team as a code owner March 6, 2024 03:28

github-actions bot added the documentation label Mar 6, 2024

michaelnebel reviewed Mar 6, 2024

View reviewed changes

egregius313 force-pushed the egregius313/csharp/dataflow/sources/commandargs-and-environment branch from 386cf4d to 4fb2693 Compare March 6, 2024 18:29

michaelnebel reviewed Mar 7, 2024

View reviewed changes

csharp/ql/lib/ext/Microsoft.Extensions.Configuration.model.yml Outdated Show resolved Hide resolved

michaelnebel reviewed Mar 7, 2024

View reviewed changes

csharp/ql/test/library-tests/dataflow/flowsources/local/commandargs/CommandArgs.ql Show resolved Hide resolved

egregius313 added 7 commits March 7, 2024 12:20

Move common source kinds to "shared"

b0eb0e1

Environment variable sources

51afe12

commandargs sources

a3f6bfe

Replace Main-method parameters with ThreatModelFlowSource

ec6e173

Microsoft.Extensions.Configuration models

f8c805d

Change note

1f64f5f

Add type signature for methods with no overloads

608a3f9

egregius313 force-pushed the egregius313/csharp/dataflow/sources/commandargs-and-environment branch from 96c3346 to 608a3f9 Compare March 7, 2024 17:32

Fix ExpandEnvironmentVariables test case

7f950d8

michaelnebel reviewed Mar 8, 2024

View reviewed changes

michaelnebel approved these changes Mar 8, 2024

View reviewed changes

egregius313 merged commit e7852f5 into github:main Mar 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

C#: Add more `environment` and `commandargs` sources for the C# Standard Library #15605

C#: Add more `environment` and `commandargs` sources for the C# Standard Library #15605

Uh oh!

egregius313 commented Feb 13, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Feb 13, 2024 •

edited

Loading

csharp

Generated file changes for csharp

Uh oh!

michaelnebel left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

egregius313 commented Mar 6, 2024 •

edited

Loading

Uh oh!

egregius313 commented Mar 7, 2024

Uh oh!

Uh oh!

Uh oh!

michaelnebel Mar 8, 2024

Uh oh!

michaelnebel left a comment

Uh oh!

michaelnebel commented Mar 8, 2024

Uh oh!

michaelnebel commented Mar 8, 2024

Uh oh!

Uh oh!

C#: Add more environment and commandargs sources for the C# Standard Library #15605

C#: Add more environment and commandargs sources for the C# Standard Library #15605

Uh oh!

Conversation

egregius313 commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

csharp

Generated file changes for csharp

Uh oh!

michaelnebel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

egregius313 commented Mar 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

egregius313 commented Mar 7, 2024

Uh oh!

Uh oh!

Uh oh!

michaelnebel Mar 8, 2024

Choose a reason for hiding this comment

Uh oh!

michaelnebel left a comment

Choose a reason for hiding this comment

Uh oh!

michaelnebel commented Mar 8, 2024

Uh oh!

michaelnebel commented Mar 8, 2024

Uh oh!

Uh oh!

C#: Add more `environment` and `commandargs` sources for the C# Standard Library #15605

C#: Add more `environment` and `commandargs` sources for the C# Standard Library #15605

egregius313 commented Feb 13, 2024 •

edited

Loading

github-actions bot commented Feb 13, 2024 •

edited

Loading

egregius313 commented Mar 6, 2024 •

edited

Loading