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

[BUG] Azure.Messaging.ServiceBus 7.18.2 contains a transitive System.Text.Json vulnerability #47864

Closed
KirkMunroSagent opened this issue Jan 16, 2025 · 10 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus

Comments

@KirkMunroSagent
Copy link

Library name and version

Azure.Messaging.ServiceBus 7.18.2

Describe the bug

Azure.Messaging.ServiceBus 7.18.2 depends on Azure.Core 1.44.0 and System.Memory.Data 6.0.0, both of which in turn depend on System.Text.Json 6.0.9 that contains a known, published vulnerability. The fix is simple, release an updated Azure.Messaging.ServiceBus 7.19.0 that takes a dependency on Azure.Core 1.44.1 and System.Memory.Data 6.0.1, both of which no longer contain the System.Text.Json vulnerability. Without this fix in place, customers need to manually take dependencies on transitive packages, and that in turn introduces more management work that should be completely unnecessary.

Expected behavior

Azure.Messaging.ServiceBus should have a release available for .NET 8 and .NET 9 that does not include transitive vulnerabilities.

Actual behavior

Azure.Messaging.ServiceBus 7.18.2 is the latest version and taking it on as a dependency means taking on transitive vulnerabilities in your project that need to be worked around until 7.19.0 is released..

Reproduction Steps

Create a new C# project.
Take a dependency on Azure.Messaging.ServiceBus 7.18.2.
Use the NuGet Package Manager or dotnet list packages --include-transitive --vulnerable to see transitive vulnerabilities in your project.

Environment

Windows 11, .NET 8 or 9, Visual Studio Latest -- updated it yesterday.

@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus labels Jan 16, 2025
Copy link

Thank you for your feedback. Tagging and routing to the team member best able to assist.

@jsquire jsquire assigned jsquire and unassigned JoshLove-msft Jan 16, 2025
@jsquire
Copy link
Member

jsquire commented Jan 16, 2025

Hi @KirkMunroSagent. Thanks for reaching out and we regret that you're experiencing difficulties. Azure Service Bus does not use System.Text.Json and is not vulnerable due to the reference. If you are using functionality of System.Text.Json in your application, then bumping the version referenced by your application will hoist the version available in the environment. Likewise, if you're using another Azure library which depends on a newer version of Azure.Core or take a direct dependency within your application, the version of Core/STJ available to Service Bus will be hoisted as well.

Azure Service Bus automatically moves to the latest Azure Core version with each release. As a result, the next release will reference a newer Azure.Core.

@jsquire jsquire added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Jan 16, 2025
Copy link

Hi @KirkMunroSagent. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jan 16, 2025
@KirkMunroSagent
Copy link
Author

KirkMunroSagent commented Jan 16, 2025

@jsquire: I realize that you don't take direct references on System.Text.Json; however, you do take a direct references on Azure.Core and System.Memory.Data, and for the versions that you use in your latest package, both of those take a direct reference on a vulnerable version of System.Text.Json, and as a result the version of System.Text.Json that is used contains vulnerabilities that are already addressed. I don't use System.Text.Json directly in my project either, I'm just inheriting the dependency because I'm using Azure.Messaging.ServiceBus.

This problem should be resolved upstream, with an update posted for Azure.Messaging.ServiceBus. Suggesting that any customer that uses this package should also take direct dependencies that need to be managed, updated, and may include additional security issues of their own that Azure.Messaging.ServiceBus is not vulnerable to, that's not a solution to this problem...that's just passing the buck.

Of course the next time you release this will be resolved. My point is you should ship a release to resolve this problem. It's responsible package management.

If you want to prevent customers like myself from asking for an unplanned update to deal with such issues, you should consider using floating version dependencies, as has been suggested in a similar thread here.

/unresolve

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. labels Jan 16, 2025
@jsquire
Copy link
Member

jsquire commented Jan 17, 2025

This problem should be resolved upstream, with an update posted for Azure.Messaging.ServiceBus. Suggesting that any customer that uses this package should also take direct dependencies that need to be managed, updated, and may include additional security issues of their own that Azure.Messaging.ServiceBus is not vulnerable to, that's not a solution to this problem...that's just passing the buck.

We appreciate your perspective. The Azure SDK team follows the guidance provided by the .NET security team for managing this scenario and the same process used by .NET for BCL libraries. For this scenario - a transitive dependency that is not used and cannot be exploited is flagged for a vulnerability - that guidance is to advise applications to take a direct reference and follow a standard release cadence rather than an immediate hotfix. Having read through the thread that you linked, the same guidance was applied there.

I do acknowledge that the guidance has friction with the changes to the way that transitive vulnerabilities are scanned in the .NET 9 SDK. It is not a good look to have Microsoft tools flagging Microsoft libraries, and we have an open inquiry with the .NET security team on that topic.

you should consider using floating version dependencies,

We appreciate your suggestion, but floating dependencies is something that we cannot adopt, as it breaks the idempotency of a release tag. We would no longer be able to count on being able to build a release target and have confidence that we didn't introduce unintended changes that would not have been rigorously tested and may break applications.

@jsquire
Copy link
Member

jsquire commented Jan 17, 2025

The good news is that as I was looking over the Service Bus schedule for releases, I noticed that there were a set of changes scheduled for our January window. These were skipped over in their normal window due to holiday schedules. As a result, Service Bus 7.18.3 was released earlier today which brings a transitive upgrade for System.Text.Json 6.0.10 along with it.

@jsquire jsquire added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Jan 17, 2025
Copy link

Hi @KirkMunroSagent. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jan 17, 2025
@KirkMunroSagent
Copy link
Author

@jsquire: I really appreciate your time, and the detailed response. It's very helpful, and truly appreciated.

I spend a lot of time on package management, and I'm always looking to validate and adjust my perspective as necessary. My suggestion for floating dependencies came only after I learned about them when was looking for some way that might work both for Microsoft teams and for consumers of Microsoft packages, and this part of your response brings up a question:

We appreciate your suggestion, but floating dependencies is something that we cannot adopt, as it breaks the idempotency of a release tag. We would no longer be able to count on being able to build a release target and have confidence that we didn't introduce unintended changes that would not have been rigorously tested and may break applications.

On one hand, I've heard a few times, here included, about a recommendation to advise applications to take on a direct reference, that is otherwise unnecessary, to resolve the flagged vulnerability. Taking on that reference would in essence result in said application running with the updated Nuget package version. On the other hand, I've heard some resistance to floating dependencies because it would introduce potentially untested combinations of Nuget packages. Those two seem to work directly against one another though, don't they? It's pushing the fix to the vulnerability out to leaf nodes (individual applications) where each application ends up running the "parent" Nuget package (the one with the transitive vulnerability) with the updated package that doesn't contain the vulnerability, so the security team recommendation is expecting that those will work together, but the project team doesn't want a transitive package dependency because they might not work together and because they haven't done their rigorous testing with those packages? Does that make sense? I've struggled with that a little bit myself, so just putting the thoughts/question out there seeking some clarity because those two approaches seem to go against one another, but maybe I'm misunderstanding something in the process.

@jsquire
Copy link
Member

jsquire commented Jan 18, 2025

@KirkMunroSagent : My pleasure and thank you for the kind words. What follows are my personal thoughts on some of the floating version topics that you've raised. Please be aware that I'm not speaking authoritatively nor on behalf of any official Microsoft stance.

On the other hand, I've heard some resistance to floating dependencies because it would introduce potentially untested combinations of Nuget packages. Those two seem to work directly against one another though, don't they?

You're not wrong; they are somewhat at odds, and I would agree that it is essentially pushing the risk down to individual applications. At the end of the day, doing so relies on the confidence that the authors of the transitive dependency have a high bar for quality and ensure strict backwards compatibility.

Where I see the key difference is scope.

Using the Azure SDK packages as an example, we have a very wide compatibility area covering all supported .NET runtimes and some niche environments with strong dependency opinions/requirements like PowerShell and Azure Functions. There are a lot of strange corner cases and nuances that we have to verify to ensure that we maintain consistent behavior and don't break existing applications. In our packages and especially in many of the official .NET runtime packages that we have dependencies on, this manifests as a number of compiler directives for conditional compilation based on target platform. The result is a lot of branched code paths that could go sideways.

Contrast that with an application - the majority have a single framework target, though they may run on different environments. The burden of ensuring that a given .NET runtime target behaves consistently across that landscape falls on the. NET runtime team. Applications can safely expect that if they run against net8.0 on Windows and Linux hosts, the application will behave the same. When they change a package version to mitigate a dependency issue, they generally know exactly what application scenarios need to be tested to ensure confidence that the app continues to function correctly.

Granted - there are places where that assumption breaks down. For example, when you're developing a library rather than an application. Then, truly, we've pushed the problem onto someone else with the same considerations and now they need to make the decision to follow suit or retest and publish an update. It also causes issues when a .NET runtime package update has an impactful bug or intentionally makes a breaking change - though I think it's fair to say that would be an issue no matter who does the bump.

Other considerations

Speaking as a library author, it is generally hard - very hard - to get the consumers of your packages to update regularly. This is particularly prevalent in the Azure ecosystem where the past generations were not centrally governed and, frankly, many package owners did a poor job ensuring backwards compatibility and adhering to SemVer. As a result, the developer community lost trust in the safety of upgrades and learned to just stick with what version worked for them.

Though the current generation is centrally managed and has those strong back-compat guarantees, people are still hesitant to upgrade, especially many of our large enterprise customers. Aside from "I don't want you to break me" the most common answer given is "you publish too frequently and there are too many version changes. Every time I do move to a new version, another one appears." In consideration of that feedback, we try to keep a consistent release cadence and push a hotfix approach only when there's an exploitable vulnerability for that package. The hope is that by doing so, people understand that it's a mitigation for an active risk.

I've struggled with that a little bit myself, so just putting the thoughts/question out there seeking some clarity because those two approaches seem to go against one another, but maybe I'm misunderstanding something in the process.

It's never an easy decision and rarely does one-size-fit-all. The changes to the .NET 9 SDK scanning approach exacerbate this; it's great that we have tools for greater awareness, but when it flags a Microsoft library and we're following standard guidance, it's hard not to feel like you're sending the message of just dismissing legitimate concerns.

Copy link

Hi @KirkMunroSagent, since you haven’t asked that we /unresolve the issue, we’ll close this out. If you believe further discussion is needed, please add a comment /unresolve to reopen the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus
Projects
None yet
Development

No branches or pull requests

3 participants