Skip to content

C#/Java: Improve Sink and Summary model generation. #16701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 14, 2024

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Jun 7, 2024

In this PR we fix to model generator bugs

  • The sink model generator didn't perceive reading of fields (and properties for C#) as additional flow steps. This has been fixed.
  • For C# we also want to perceive property/reads as additional flow steps for source and sink extrapolation and we also want the summary generation to take property reads and writes into account. This has been fixed as well.

There has also been some errors lately with catastrophic evaluator crashes when running the model generator. The is a known issue by the foundations team; As a workaround we increase the allowed memory consumption to 32GB.

As a side effect of these changes in this PR, sink model generation is now observably idempotent for .NET Runtime.

@github-actions github-actions bot added the C# label Jun 7, 2024
Copy link
Contributor

github-actions bot commented Jun 7, 2024

⚠️ 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``",44,9873,49,9
+    System,"``System.*``, ``System``",44,10614,60,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``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",54,1357,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.DotNet.Build.Tasks``, ``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``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",54,1821,148,
-    Totals,,98,11237,391,9
+    Totals,,98,12442,402,9
  • Changes to framework-coverage-csharp.csv:
- ILCompiler,,,81,,,,,,,,,,,,,,,,,,,81,
+ ILCompiler,,,123,,,,,,,,,,,,,,,,,,,123,
- ILLink.RoslynAnalyzer,,,63,,,,,,,,,,,,,,,,,,,63,
+ ILLink.RoslynAnalyzer,,,145,,,,,,,,,,,,,,,,,,,145,
- ILLink.Shared,,,32,,,,,,,,,,,,,,,,,,,30,2
+ ILLink.Shared,,,34,,,,,,,,,,,,,,,,,,,32,2
- ILLink.Tasks,,,3,,,,,,,,,,,,,,,,,,,3,
+ ILLink.Tasks,,,4,,,,,,,,,,,,,,,,,,,4,
- Internal.TypeSystem,,,291,,,,,,,,,,,,,,,,,,,275,16
+ Internal.TypeSystem,,,315,,,,,,,,,,,,,,,,,,,299,16
- JsonToItemsTaskFactory,,,5,,,,,,,,,,,,,,,,,,,5,
+ JsonToItemsTaskFactory,,,10,,,,,,,,,,,,,,,,,,,10,
- Microsoft.Android.Build,,,14,,,,,,,,,,,,,,,,,,,14,
+ Microsoft.Android.Build,,,16,,,,,,,,,,,,,,,,,,,16,
- Microsoft.Apple.Build,,,5,,,,,,,,,,,,,,,,,,,5,
+ Microsoft.Apple.Build,,,8,,,,,,,,,,,,,,,,,,,8,
- Microsoft.CSharp,,,10,,,,,,,,,,,,,,,,,,,10,
+ Microsoft.CSharp,,,13,,,,,,,,,,,,,,,,,,,13,
+ Microsoft.DotNet.Build.Tasks,,,6,,,,,,,,,,,,,,,,,,,6,
- Microsoft.Extensions.Caching.Distributed,,,9,,,,,,,,,,,,,,,,,,,9,
+ Microsoft.Extensions.Caching.Distributed,,,10,,,,,,,,,,,,,,,,,,,10,
- Microsoft.Extensions.Caching.Memory,,,30,,,,,,,,,,,,,,,,,,,29,1
+ Microsoft.Extensions.Caching.Memory,,,39,,,,,,,,,,,,,,,,,,,38,1
- Microsoft.Extensions.Configuration,,2,77,,,,,,,,,,,,,2,,,,,,76,1
+ Microsoft.Extensions.Configuration,,2,90,,,,,,,,,,,,,2,,,,,,89,1
- Microsoft.Extensions.DependencyInjection,,,96,,,,,,,,,,,,,,,,,,,95,1
+ Microsoft.Extensions.DependencyInjection,,,134,,,,,,,,,,,,,,,,,,,133,1
- Microsoft.Extensions.DependencyModel,,,9,,,,,,,,,,,,,,,,,,,9,
+ Microsoft.Extensions.DependencyModel,,,18,,,,,,,,,,,,,,,,,,,18,
- Microsoft.Extensions.FileSystemGlobbing,,,16,,,,,,,,,,,,,,,,,,,14,2
+ Microsoft.Extensions.FileSystemGlobbing,,,18,,,,,,,,,,,,,,,,,,,16,2
- Microsoft.Extensions.Hosting,,,26,,,,,,,,,,,,,,,,,,,25,1
+ Microsoft.Extensions.Hosting,,,41,,,,,,,,,,,,,,,,,,,40,1
- Microsoft.Extensions.Http,,,8,,,,,,,,,,,,,,,,,,,8,
+ Microsoft.Extensions.Http,,,9,,,,,,,,,,,,,,,,,,,9,
- Microsoft.Extensions.Logging,,,53,,,,,,,,,,,,,,,,,,,52,1
+ Microsoft.Extensions.Logging,,,65,,,,,,,,,,,,,,,,,,,64,1
- Microsoft.Extensions.Options,,,8,,,,,,,,,,,,,,,,,,,8,
+ Microsoft.Extensions.Options,,,13,,,,,,,,,,,,,,,,,,,13,
- Microsoft.Extensions.Primitives,,,64,,,,,,,,,,,,,,,,,,,64,
+ Microsoft.Extensions.Primitives,,,72,,,,,,,,,,,,,,,,,,,72,
- Microsoft.Interop,,,73,,,,,,,,,,,,,,,,,,,73,
+ Microsoft.Interop,,,121,,,,,,,,,,,,,,,,,,,121,
- Microsoft.NET.Build.Tasks,,,1,,,,,,,,,,,,,,,,,,,1,
+ Microsoft.NET.Build.Tasks,,,4,,,,,,,,,,,,,,,,,,,4,
- Microsoft.NET.WebAssembly.Webcil,,,7,,,,,,,,,,,,,,,,,,,7,
+ Microsoft.NET.WebAssembly.Webcil,,,8,,,,,,,,,,,,,,,,,,,8,
- Microsoft.WebAssembly.Build.Tasks,,,3,,,,,,,,,,,,,,,,,,,3,
+ Microsoft.WebAssembly.Build.Tasks,,,4,,,,,,,,,,,,,,,,,,,4,
- Mono.Linker,,,158,,,,,,,,,,,,,,,,,,,158,
+ Mono.Linker,,,285,,,,,,,,,,,,,,,,,,,285,
- SourceGenerators,,,4,,,,,,,,,,,,,,,,,,,4,
+ SourceGenerators,,,5,,,,,,,,,,,,,,,,,,,5,
- System,49,44,9873,,3,3,1,,,4,5,,33,2,,3,15,17,3,4,,7968,1905
+ System,60,44,10614,,7,6,5,,,4,5,,33,2,,3,15,17,3,4,,8709,1905

@michaelnebel michaelnebel force-pushed the csharp/modelgentaintmembers branch from 57a6457 to deacdeb Compare June 10, 2024 08:16
@michaelnebel michaelnebel added the no-change-note-required This PR does not need a change note label Jun 10, 2024
@michaelnebel michaelnebel marked this pull request as ready for review June 10, 2024 08:59
@michaelnebel michaelnebel requested a review from a team as a code owner June 10, 2024 08:59
@michaelnebel michaelnebel force-pushed the csharp/modelgentaintmembers branch from deacdeb to 9ea2b46 Compare June 13, 2024 06:36
@michaelnebel michaelnebel changed the title C#: Model generator - taint members. C#: Model generator - Improve Sink model generation. Jun 13, 2024
@michaelnebel michaelnebel changed the title C#: Model generator - Improve Sink model generation. C#: Improve Sink and Summary model generation. Jun 13, 2024
@michaelnebel michaelnebel changed the title C#: Improve Sink and Summary model generation. C#/Java: Improve Sink and Summary model generation. Jun 13, 2024
@michaelnebel michaelnebel marked this pull request as draft June 13, 2024 08:54
@github-actions github-actions bot added the Java label Jun 13, 2024
@michaelnebel michaelnebel requested a review from hvitved June 13, 2024 09:03
@michaelnebel
Copy link
Contributor Author

DCA looks good.
There are 3 new results: I spot checked the extra result for cs/web/xss on jerryhoff and the results shows up due to the added summary

["System.Net", "WebHeaderCollection", False, "ToString", "()", "", "Argument[this]", "ReturnValue", "taint", "df-generated"]

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some questions.

@michaelnebel michaelnebel requested a review from hvitved June 14, 2024 09:07
@michaelnebel michaelnebel marked this pull request as ready for review June 14, 2024 09:07
@michaelnebel michaelnebel requested a review from a team as a code owner June 14, 2024 09:07
@michaelnebel michaelnebel merged commit 3525967 into github:main Jun 14, 2024
@michaelnebel michaelnebel deleted the csharp/modelgentaintmembers branch June 14, 2024 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# Java no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants