Skip to content

Fix known high severity vulnerabilities - #2613

Merged
timcassell merged 2 commits into
masterfrom
vulns
Oct 21, 2024
Merged

Fix known high severity vulnerabilities#2613
timcassell merged 2 commits into
masterfrom
vulns

Conversation

@adamsitnik

@adamsitnik adamsitnik commented Aug 12, 2024

Copy link
Copy Markdown
Member

I've tried to build and run our samples today and got following errors:

PS D:\projects\BenchmarkDotNet\samples\BenchmarkDotNet.Samples> dotnet run -c Release -f net8.0 --filter *Counters* --list flat
C:\Program Files\dotnet\sdk\9.0.100-preview.7.24402.8\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.TargetFrameworkInference.targets(187,5): warning NETSDK1215: Targeting .NET Standard prior to 2.0 is no longer recommended. See https://aka.ms/dotnet/dotnet-standard-guidance for more details.
D:\projects\BenchmarkDotNet\src\BenchmarkDotNet.Annotations\BenchmarkDotNet.Annotations.csproj : error NU1903: Warning As Error: Package 'System.Text.RegularExpressions' 4.3.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-cmhx-cq75-c4mj
D:\projects\BenchmarkDotNet\samples\BenchmarkDotNet.Samples\BenchmarkDotNet.Samples.csproj : error NU1903: Warning As Error: Package 'System.Net.Http' 4.3.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-7jgj-8wvc-jh57
D:\projects\BenchmarkDotNet\samples\BenchmarkDotNet.Samples\BenchmarkDotNet.Samples.csproj : error NU1903: Warning As Error: Package 'System.Text.RegularExpressions' 4.3.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-cmhx-cq75-c4mj
D:\projects\BenchmarkDotNet\src\BenchmarkDotNet.Diagnostics.dotMemory\BenchmarkDotNet.Diagnostics.dotMemory.csproj : error NU1903: Warning As Error: Package 'System.Net.Http' 4.3.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-7jgj-8wvc-jh57
D:\projects\BenchmarkDotNet\src\BenchmarkDotNet.Diagnostics.dotTrace\BenchmarkDotNet.Diagnostics.dotTrace.csproj : error NU1903: Warning As Error: Package 'System.Net.Http' 4.3.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-7jgj-8wvc-jh57
D:\projects\BenchmarkDotNet\src\BenchmarkDotNet.Diagnostics.dotMemory\BenchmarkDotNet.Diagnostics.dotMemory.csproj : error NU1903: Warning As Error: Package 'System.Text.RegularExpressions' 4.3.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-cmhx-cq75-c4mj
D:\projects\BenchmarkDotNet\src\BenchmarkDotNet.Diagnostics.dotTrace\BenchmarkDotNet.Diagnostics.dotTrace.csproj : error NU1903: Warning As Error: Package 'System.Text.RegularExpressions' 4.3.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-cmhx-cq75-c4mj

My first thought was that the fix will be straightforward: just update System.Text.RegularExpressions and System.Net.Http to most recent versions. I've quickly realized that these are transitive dependencies.

In case of BenchmarkDotNet.Annotations, this project targets netstandard1.0 (it's just a project with attributes, almost no logic at all and we wanted to target lowest tfm possible). This gives us a dependency to https://www.nuget.org/packages/NETStandard.Library/1.6.1. Can we just update it? No, because it has not been updated since 2018. The following warning suggests that it's on purpose:

warning NETSDK1215: Targeting .NET Standard prior to 2.0 is no longer recommended.

I could just remove the netstandard1.0 TFM from BenchmarkDotNet.Annotations, but BenchmarkDotNet.Diagnostics.dotTrace and BenchmarkDotNet.Diagnostics.dotMemory both depend on https://www.nuget.org/packages/JetBrains.Profiler.SelfApi/, which depends on https://www.nuget.org/packages/JetBrains.HabitatDetector/ which depends on https://www.nuget.org/packages/JetBrains.FormatRipper/ which has the same dependency:

image

As a quick workaround I've decided to just add a dependency to these two packages (System.Text.RegularExpressions and System.Net.Http) to BenchmarkDotNet.Annotations, which all BDN packages depend on.

The alternatives I've considered:

cc @ericstj

@adamsitnik adamsitnik added this to the v0.14.1 milestone Aug 12, 2024
@adamsitnik

Copy link
Copy Markdown
Member Author

Open an issue and send a PR to https://www.nuget.org/packages/JetBrains.Profiler.SelfApi/ to change the supported monikers: from net46 to net462 (this would pick up the netstandard2.0 dependency of JetBrains.HabitatDetector and solve the problem).

I gave it a try and it has not solved the problem. Considering https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-0 the right solution would be to remove netstandard1.x support in all the mentioned packages?

@Numpsy

Numpsy commented Aug 12, 2024

Copy link
Copy Markdown

I don't know if it'd help for this situation, but for the

In case of BenchmarkDotNet.Annotations

case, if you wanted to keep the old .NET Standard 1.x version, I think some of the Serilog libraries used to set

<DisableImplicitFrameworkReferences>true</DisableImplicitFrameworkReferences>

for .NET Standard 1.x targets to remove the automatic dependency on NETStandard.Library and then directly reference the minimum bits that they actually use (and then the references to Http and RegularExpression might go away altogether, rather than needing to be updated)

@ericstj ericstj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you can, get all your dependencies to, at the very least, add new target frameworks that are newer than netstandard1.x. When targeting netstandard2.0 or newer no framework packages are referenced at all. Ideally they'd remove the netstandard1.x targets as well - but that's not necessary to expose fewer dependencies to consumers.

If you can't get your dependencies to do that, then updating to the latest version of NETStandard.Library may solve the issue without needing to add unused dependencies since the latest version of this package will avoid bringing in the old packages on frameowrks where they aren't needed.

Comment thread samples/BenchmarkDotNet.Samples.FSharp/BenchmarkDotNet.Samples.FSharp.fsproj Outdated
Comment thread src/BenchmarkDotNet.Annotations/BenchmarkDotNet.Annotations.csproj Outdated
@timcassell

This comment was marked as resolved.

@AndreyAkinshin

Copy link
Copy Markdown
Member

I vote for dropping netstandard1.0. This target brings a lot of problems (and I expect more problems in the future), while there is not much value in supporting obsolete runtimes.

Speaking of JetBrains.FormatRipper, the netstandard2.0 target was added. Hopefully, it will solve the transitive dependency problems in dotTrace/dotMemory packages (once the new versions of the JetBraind dependencies are available).

adamsitnik and others added 2 commits October 21, 2024 14:37
Updated dependencies to fix vulnerabilities.
Removed netstandard1.0 target from Annotations.
@timcassell timcassell changed the title workaround known high severity vulnerabilities Fix known high severity vulnerabilities Oct 21, 2024
@timcassell
timcassell merged commit af8bde4 into master Oct 21, 2024
@timcassell
timcassell deleted the vulns branch October 21, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants