Skip to content
New issue

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

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

Already on GitHub? # to your account

Indirect dependency on vulnerable System.Text.Json 8.0.0 through Microsoft.Extensions.Configuration.Json #104705

Closed
xC0dex opened this issue Jul 11, 2024 · 10 comments

Comments

@xC0dex
Copy link

xC0dex commented Jul 11, 2024

Description

I have <NuGetAuditMode>all</NuGetAuditMode> enabled in my Blazor authentication project. I encountered a high severity vulnerability warning for System.Text.Json version 8.0.0 (Announcement). This package is indirectly installed through Microsoft.Extensions.Configuration.Json version 8.0.0 which is installed by using Microsoft.AspNetCore.Components.WebAssembly version 8.0.7.
I'm not sure if that's the way it should be.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jul 11, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

@gregsdennis
Copy link
Contributor

8.0.4 is available. Do you get the warning if you explicitly install that package?

@xC0dex
Copy link
Author

xC0dex commented Jul 11, 2024

8.0.4 is available. Do you get the warning if you explicitly install that package?

I forgot to mention this. The warning is no longer there when I install 8.0.4 directly. I wondered why vulnerable packages are indirectly installed and whether this is intentional. I just wanted to bring this issue to your attention.

@xC0dex
Copy link
Author

xC0dex commented Jul 11, 2024

Ups, I just saw that #104619 would be the right place for mentioning this, sorry!

@eiriktsarpalis
Copy link
Member

Duplicate of #104669. We don't typically update packages because of a vulnerable dependency. The recommendation is to explicitly reference the patched STJ version.

@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jul 11, 2024
@julealgon
Copy link

The recommendation is to explicitly reference the patched STJ version.

@eiriktsarpalis shouldn't NuGet give lower priority to vulnerable packages when auto-resolving a transitive dependency though?

Why would it explicitly pick a vulnerable version if there is a newer patch version that is not vulnerable that fits the version range criteria for the parent?

If the user doesn't explicitly install the transitive package (which the vast majority of people won't, especially if it's various levels of transitivity deep) then NuGet should just have a safer default.

@eiriktsarpalis
Copy link
Member

shouldn't NuGet give lower priority to vulnerable packages when auto-resolving a transitive dependency though?

I don't think this should be done automatically, primarily because NuGet wouldn't be able to determine if the next version contains breaking changes. It should be up to the user to either suppress the warning or explicitly bump the transitive dependency.

@Clockwork-Muse
Copy link
Contributor

if there is a newer patch version that is not vulnerable that fits the version range criteria for the parent?

This should be viable, though, if NuGet had some way to tell the package was vulnerable (or the repository didn't return the vulnerable versions for range searches)? If we excluded the affected versions from the list the normal resolution should take over?

@julealgon
Copy link

Another thing to keep in mind here is that maintaining the indirect dependencies explicitly adds a ton of maintenance burden on consumers.

I just went through a process where we shifted from the old packages.config way of handling dependencies (where you needed to explicitly reference the entire dependency graph), to the modern PackageReference approach where transitive dependencies can be inferred. We painstakingly streamlined our list of dependencies to only have what we really needed directly while allowing NuGet to automatically resolve transitive ones.

I'd rather not go back and have to maintain transitive dependencies again explicitly. I think that defeats a significant purpose of using PackageReferences in the first place.

If a package version is vulnerable, NuGet should remove that from the pool of "available choices" when selecting transitive dependencies automatically, and only in the case that there is no non-vulnerable replacement should it fallback to adding the vulnerable package with a warning.

@eiriktsarpalis
Copy link
Member

I think this would be an issue to bring up with the NuGet team. My main concern is that versioning semantics are not consistent for all maintainers. NuGet should not implicitly assume that a patch version increment is compatible.

I'd rather not go back and have to maintain transitive dependencies again explicitly.

That's understandable but to be fair central package management does alleviate that burden to an extent.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

5 participants