serilog / serilog-settings-configuration

A Serilog configuration provider that reads from Microsoft.Extensions.Configuration
Apache License 2.0
446 stars 129 forks source link

Insecure defaults through Newtonsoft.Json vulnerability #341

Closed ThomasBergholdWieser closed 2 months ago

ThomasBergholdWieser commented 1 year ago

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 -> Serilog.Settings.Configuration@3.4.0 › Microsoft.Extensions.DependencyModel@3.0.0 › Newtonsoft.Json@9.0.1

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 commented 1 year ago

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 https://github.com/serilog/serilog-settings-configuration/pull/339 so with some luck the experience will be smoother in future.

ThomasBergholdWieser commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

fixed in #351

Numpsy commented 2 months ago

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