Updated Newtonsoft.Json from 10.0.2 to latest to fix security vulnerability.

[brenordv]

Pull Request Template

Description

This project referenced an outdated version of Newtonsoft.Json (10.0.2) which had a vulnerability introduced on 24 Apr 2022.

For context, quoting the description from the nist.org page:

Newtonsoft.Json before version 13.0.1 is affected by a mishandling of exceptional conditions vulnerability. Crafted data that is passed to the JsonConvert.DeserializeObject method may trigger a StackOverflow exception resulting in denial of service. Depending on the usage of the library, an unauthenticated and remote attacker may be able to cause the denial of service condition.

For consistency, other projects that also use Newtonsoft.Json were brought up to the same version.

No other changes were made to the code.

References

• Snyk: https://security.snyk.io/vuln/SNYK-DOTNET-NEWTONSOFTJSON-2774678
• CVE- 2024-21907: https://www.cve.org/CVERecord?id=CVE-2024-21907 / https://nvd.nist.gov/vuln/detail/CVE-2024-21907
• CWE-755: https://cwe.mitre.org/data/definitions/755.html

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

Closing issues

To automatically close an issue: closes #IssueNumber

[FabianMeiswinkel]

This upgrade cannot be made because dependency upgrades with major version difference are considered breaking. And this would require a new major version of the SDK.

[brenordv]

@FabianMeiswinkel Alright. Makes sense.
Is there any plan to make those updates?
This project also relies on vulnerable versions of System.Net.Http, and System.Text.RegularExpressions.

[FabianMeiswinkel]

This upgrade cannot be made (at least in production code Microsoft.Azure.Cosmos and Microsoft.Azure.Cosmos.Encryption) because dependency upgrades with major version difference are considered breaking. And this would require a new major version of the SDK.

[brenordv]

Considering the point made by @FabianMeiswinkel, I'm closing this PR, since it wouldn't make much sense to update some, but not all the package versions, because the overall project would still be left vulnerable, and still generate alerts by code analyzers (such as Snyk, and SonarCloud).

[RockyMM]

Hi all involved,

Is there a plan on how to treat vulnerable dependency versions?

To be frank, I am not sure why would a major dependency version upgrade mean also a major release for the SDK. But besides this point, Newtonsoft.Json does not really adhere to the semver, as its minor version is always "0". So upgrading from 10.0.2 to 13.0.3 is not a major version upgrade in a semver sense.

[ealsur]

@RockyMM the problem is that between 10.0.2 and 13, there were breaking changes in Newtonsoft.Json.

We cannot bump a dependency that has breaking changes on a minor version upgrade of the SDK.

[Lucasharskamp]

@RockyMM the problem is that between 10.0.2 and 13, there were breaking changes in Newtonsoft.Json.

We cannot bump a dependency that has breaking changes on a minor version upgrade of the SDK.

Hello,

In my opinion, the problem is two-fold:

1. the library uses vulnerable versions of existing libraries, mentioned are Newtonsoft/System.Net.Http/System.Text.RegularExpressions and IIRC there is another library as well that has issues.
2. the library works from the assumption that we want to use Newtonsoft.JSON to serialize, rather than system.text.json or something else entirely. In my opinion, a NuGet package shouldn't include items the end user doesn't necessarily need.

In my employer's implementation of the Cosmos library, we use system.text.json for everything rather than Newtonsoft. We also use a custom CosmosSerializer to get that to happen. But, because of this library, we still carry a needless (!) faulty version of the library in our builds. We also probably use some of those vulnerabilities from the other libraries. Granted, the code on production goes from an Azure Web App to Cosmos and back on the same region using a connectionstring acquired from a KeyVault, so I doubt anything can go wrong, but still.

If possible, could it be done that a major revision will get in the works for a quick release with the intent to:

1. fix the library issues
2. allow the end user to choose their JSON usage, with very small NuGet packages that contain nothing more than a Newtonsoft/system.text.json serializer. (the end user can also decide to use neither to allow one to create their own custom CustomSerializer)

Thank you!

[ealsur]

@Lucasharskamp

1. The library uses Newtonsoft.Json, even if you use System.Text.Json or other custom serialization engine for your Data Plane operations, Newtonsoft.Json is used for metadata and service calls.
2. If you read the Security Notice it says that upgrading is just one way to fix the security issue.

To mitigate the issue one either need to update Newtonsoft.Json to 13.0.1 or set MaxDepth parameter in the JsonSerializerSettings. This can be done globally with the following statement.

We cannot upgrade the dependency to 13.0.1 because it poses breaking changes to customers. Following SemVer, the only way to push a change that creates a breaking change is to bump the Major Version, so this is only possible in a 4.X SDK, not 3.X.

We already fixed the library issue, every place the library uses Newtonsoft.Json, has the MaxDepth set. So from the security perspective, the library is secured. If your application does not use Newtonsoft.Json, then based on the Security Notice, your application is secured. If there is tooling that decided to enforce nuget package versioning when the security notice clearly says that that is only one of the options, that seems to be a gap in the tooling analysis. If your application uses Newtonsoft.Json, then you can decide to either bump to 13.0.1 and address the breaking changes or use MaxDepth in the places you interact with Newtonsoft.Json.

[RockyMM]

@ealsur Thank you for the explanation. I will relay your explanation to the team. We will review our usage of Newtonsoft.Json and do we use MaxDepth property.

Still, hoping for a sooner than later release of 4.y.z SDK 🙌

[Lucasharskamp]

@ealsur Thank you! That resolved my issue as well, just putting the Newtonsoft.Json most recent version together in the same assembly as where I azure Azure.Cosmos did the trick.