Skip to content
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

Update Microsoft.CodeAnalysis version to 4.10 #34116

Closed
wants to merge 1 commit into from

Conversation

halter73
Copy link
Member

This should help avoid warnings due to the transitive System.Drawing.Common 4.7.0 dependency that has a "critical" CVE for an RCE vulnerability. GHSA-rxg9-xrhp-64gj.

Right now, System.Drawing.Common is transitively referenced via Microsoft.CodeAnalysis.Workspaces.MSBuild 4.8.0 -> Microsoft.Build.Framework 16.10.0 -> System.Security.Permissions 4.7.0 -> System.Windows.Extensions 4.7.0 -> System.Drawing.Common 4.7.0.

I think updating the Microsoft.CodeAnalysis.Workspaces.MSBuild dependency from 4.8.0 to 4.10.0 should remove the transitive System.Drawing.Common dependency entirely.

@halter73
Copy link
Member Author

  • @roji since it seems like you wanted to hold off on updating earlier. Hopefully this is okay now that we have the final release of 4.10.

@roji
Copy link
Member

roji commented Jul 1, 2024

Thanks @halter73. This causes some analyzer-related test failures:

System.TypeInitializationException
The type initializer for 'Microsoft.CodeAnalysis.Testing.Lightup.ProgrammaticSuppressionInfoWrapper' threw an exception.
   at Microsoft.CodeAnalysis.Testing.Lightup.ProgrammaticSuppressionInfoWrapper.FromInstance(Object instance) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/Lightup/ProgrammaticSuppressionInfoWrapper.cs:line 32
   at Microsoft.CodeAnalysis.Testing.Extensions.DiagnosticExtensions.ProgrammaticSuppressionInfo(Diagnostic diagnostic) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/Extensions/DiagnosticExtensions.cs:line 38
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.<>c.<FilterDiagnostics>b__103_0(ValueTuple`2 d) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 1687
   at System.Linq.Enumerable.ArrayWhereIterator`1.ToArray(ReadOnlySpan`1 source, Func`2 predicate)
   at System.Linq.Enumerable.ArrayWhereIterator`1.ToArray()
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at System.Collections.Immutable.ImmutableArray.CreateRange[T](IEnumerable`1 items)
   at System.Collections.Immutable.ImmutableArray.ToImmutableArray[TSource](IEnumerable`1 items)
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.FilterDiagnostics(ImmutableArray`1 diagnostics) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 1686
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.GetSortedDiagnosticsAsync(Solution solution, ImmutableArray`1 analyzers, ImmutableArray`1 additionalDiagnostics, CompilerDiagnostics compilerDiagnostics, IVerifier verifier, CancellationToken cancellationToken) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 1144
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.GetSortedDiagnosticsAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, ImmutableArray`1 analyzers, IVerifier verifier, CancellationToken cancellationToken) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 1090
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.VerifyDiagnosticsAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, DiagnosticResult[] expected, IVerifier verifier, CancellationToken cancellationToken) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 449
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.RunImplAsync(CancellationToken cancellationToken) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 203
   at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.RunAsync(CancellationToken cancellationToken) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 172
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass46_0.<<InvokeTestMethodAsync>b__1>d.MoveNext() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 253
--- End of stack trace from previous location ---
   at Xunit.Sdk.ExecutionTimer.AggregateAsync(Func`1 asyncAction) in /_/src/xunit.execution/Sdk/Frameworks/ExecutionTimer.cs:line 48
   at Xunit.Sdk.ExceptionAggregator.RunAsync(Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 90

System.InvalidOperationException
Property 'System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.Diagnostics.Suppression] Suppressions' produces a value of type 'System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.Diagnostics.Suppression]', which is not assignable to type 'System.Collections.Immutable.ImmutableHashSet`1[System.ValueTuple`2[System.String,Microsoft.CodeAnalysis.LocalizableString]]'
   at Microsoft.CodeAnalysis.Testing.Lightup.LightupHelpers.CreatePropertyAccessor[T,TResult](Type type, String propertyName, TResult defaultValue) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/Lightup/LightupHelpers.cs:line 53
   at Microsoft.CodeAnalysis.Testing.Lightup.ProgrammaticSuppressionInfoWrapper..cctor() in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/Lightup/ProgrammaticSuppressionInfoWrapper.cs:line 21

Any idea what these are about?

@roji
Copy link
Member

roji commented Jul 1, 2024

Also, just noting that switching to version 4.10 requires using VS version Visual Studio 2022 version 17.10 (which shouldn't be a problem, see version docs).

@AndriySvyryd
Copy link
Member

I think MicrosoftCodeAnalysisTestingVersion needs to be updated too

eng/Versions.props Outdated Show resolved Hide resolved
@AndriySvyryd AndriySvyryd self-requested a review August 12, 2024 20:53
@AndriySvyryd
Copy link
Member

We decided not to take this change in 9.0, see #33970
From what I can tell, the way the transitive dependency is used doesn't expose the user to the vulnerability.

@roji roji force-pushed the halter73/update-codeanalysis-version branch from 4888d98 to 9d2b31e Compare August 13, 2024 06:04
This should help avoid warnings due to the transitive System.Drawing.Common 4.7.0 dependency that has a "critical" CVE for an RCE vulnerability. GHSA-rxg9-xrhp-64gj.

Right now, System.Drawing.Common is transitively referenced via Microsoft.CodeAnalysis.Workspaces.MSBuild 4.8.0 -> Microsoft.Build.Framework 16.10.0 -> System.Security.Permissions 4.7.0 -> System.Windows.Extensions 4.7.0 -> System.Drawing.Common 4.7.0.

I think updating the Microsoft.CodeAnalysis.Workspaces.MSBuild dependency from 4.8.0 to 4.10.0 should remove the transitive System.Drawing.Common dependency entirely.
@roji
Copy link
Member

roji commented Aug 13, 2024

The test errors after upgrading to 4.10.0 are already tracked by dotnet/roslyn-sdk#1175

@roji
Copy link
Member

roji commented Aug 13, 2024

Noting that the CVE here against the transitive dependency of Microsoft.CodeAnalysis 4.8.0 isn't relevant, as it's a private asset of our Microsoft.Analyzers package.

I'll go ahead and close this for now, and we can revisit for EF 10 whenever we need to upgrade the package for whatever reason.

@roji roji closed this Aug 13, 2024
@roji roji deleted the halter73/update-codeanalysis-version branch August 13, 2024 15:00
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.

3 participants