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

Insecure defaults through Newtonsoft.Json vulnerability #341

Closed
ThomasBergholdWieser opened this issue Dec 1, 2022 · 6 comments
Closed

Comments

@ThomasBergholdWieser
Copy link

Our Snyk scan has revealed the following issues with the current version of the package:

The issue is regarding the currently used version of Newtonsoft.Json, which is ->
[email protected][email protected][email protected]

Affected versions of this package are vulnerable to Insecure Defaults due to improper handling of StackOverFlow exception (SOE) whenever nested expressions are being processed. Exploiting this vulnerability results in Denial Of Service (DoS), and it is exploitable when an attacker sends 5 requests that cause SOE in time frame of 5 minutes.

Note: This vulnerability is only applicable to systems deployed on IIS (Internet Information Services) web-server

The issue is fixed in Newtonsoft.Json version 13.0.1 and higher.

@nblumhardt
Copy link
Member

Hi Thomas, thanks for dropping by. The dependency on Microsoft.Extensions.DependencyModel sets a minimum version; it's 99.9% likely that if you're running on a supported .NET version with recent updates you'll get a much, much newer version of this dependency and no such vulnerability will exist in the packages actually installed/consumed by your application.

For mostly unrelated reasons we're looking at changing the way this package depends on Microsoft.Extensions.DependencyModel in #339 so with some luck the experience will be smoother in future.

@ThomasBergholdWieser
Copy link
Author

Hi, we run our project on <TargetFramework>net6.0</TargetFramework> with <LangVersion>latest</LangVersion> (not that that matters much, anyway) and these references:

<ItemGroup>
    <PackageReference Include="Serilog" Version="2.12.0" />
    <PackageReference Include="Serilog.AspNetCore" Version="6.1.0" />
    <PackageReference Include="Serilog.Enrichers.AssemblyName" Version="1.0.9" />
    <PackageReference Include="Serilog.Enrichers.CorrelationId" Version="3.0.1" />
    <PackageReference Include="Serilog.Enrichers.Environment" Version="2.2.0" />
    <PackageReference Include="Serilog.Enrichers.Process" Version="2.0.2" />
    <PackageReference Include="Serilog.Expressions" Version="3.4.1" />
    <PackageReference Include="Serilog.Settings.Configuration" Version="3.4.0" />
    <PackageReference Include="Serilog.Sinks.File" Version="5.0.0" />
</ItemGroup>

<ItemGroup>
    <FrameworkReference Include="Microsoft.AspNetCore.App" />
</ItemGroup>

I am not really sure what to change to fix this issue, as it seems we already are quite recent.

@mattheys
Copy link

mattheys commented Feb 9, 2023

I am getting this a lot as well in Snyk, Newtonsoft.Json is a very popular package and Serilog.Settings.Configuration was just the first on in the list I saw.

Adding an explicit reference to Newtonsoft.Json seems to have removed the issue for me, I'm guessing if you had another package that had a dependency on Newtonsoft.Json >= 13.0.1 it might have worked as well.

<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />

Nuget dependencies confuse me a little but what I think what nblumhardt meant was because Microsoft.Extensions.DependencyModel has a minimum >= of 9.0.1 then it will still work if something references something higher, so by explicitly setting it to 13.0.1 (which Snyk says it is resolved in) then it won't use 9.0.1 in your project.

@sungam3r
Copy link
Contributor

@nblumhardt @skomis-mm you may consider to bump Microsoft.Extensions.DependencyModel to version >= 6 (5 still references nsj for net451) in the light of upcoming serilog bump to v3. Also ping @SimonCropp

@sungam3r
Copy link
Contributor

fixed in #351

@Numpsy
Copy link
Member

Numpsy commented Jul 10, 2024

Can this be closed now? (at this point, Microsoft.Extensions.DependencyModel should just be using System.Text.Json rather than Newtonsoft.)

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

No branches or pull requests

5 participants