NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 252 forks source link

dotnet restore should report security vulnerabilities for shared assembly projects. #13708

Closed DanTravison closed 1 month ago

DanTravison commented 3 months ago

NuGet Product(s) Involved

dotnet.exe

The Elevator Pitch

When a shared assembly has a vulnerability in one of its dependencies, it should be identified explicitly. Instead, the vulnerability is reported through the containing application with a link to the vulnerability.

The problem is the suggested fix leads the dev to fix the application with a PackageReference/explicit version instead of addressing the issue directly in the shared assembly and obfuscates the fact that other uses of hte shared assembly have the same vulnerability.

Additional Context and Details

I encountered the System.Private.Uri security vulnerability today after upgrading my VS installation when building a 'test application' for testing some Maui-base shared assemblies.

At first, I thought his was an issue with the VS upgrade, since I had been building the shared assemblies and all dependent applications over the previous few days without any issues.

Next, I thought there was an issue with the test application itself, so I added the PackageReference to it.

I then started running dotnet resolve against the dependent applications and started seeing the same issue for a number of the applications.

After researching the issue, I found this thread (https://github.com/dotnet/sdk/issues/42651) and the suggestion to searching project.assets.json and found numerous applications had the same issue and eventually determined that the root cause was in one of my shared assemblies.

If dotnet resolve had identified the shared assembly in addition to, or instead of, the various application projects, it would have been much quicker to find the root cause. As it stands, I could have easily fixed on application only to discover it in others later.

zivkan commented 2 months ago

@DanTravison by "shared assembly", do you mean what Visual Studio used to call a "shared project" (.shproj)?

Those don't create separate assemblies, so if you have a console app referencing a .shproj, then there's only one assembly (.dll), the console app. There is no "shared assembly". Also, these .shproj were not supported by older versions of the .NET SDK, only by Visual Studio. Unless this has changed, then using .shproj with MAUI projects puts you outside Microsoft's supported scenarios. I know some people have gotten around it by directly <Import Project="..\path\to\project.props" />, but then from an MSBuild point of view, it's no longer two different projects. There's no feasible way to know that items defined in whatever.props is part of a different project. It's indistinguishable from being in the "child" project's csproj directly.

If by shared assembly you mean a class library project (dotnet new classlib, or Visual Studio's "Class Library" project template), then you can get this information from dotnet nuget why, and we also recently published a blog post about investigating NuGetAudit reports of vulnerable packages found during restore, where dotnet nuget why, and some other tools, were mentioned.

If neither of my guesses are correct, I don't understand your scenario. Can you please create a sample solution, zip it up, and attach it here?

ProbablePrime commented 1 month ago

Based on further conversation in: https://github.com/dotnet/sdk/issues/42651#issuecomment-2372410311, this issue should be linked to: https://github.com/NuGet/Home/issues/13718

As #13708, was generated because of https://github.com/dotnet/sdk/issues/42651

As such leaving a comment :), thanks!