serilog / serilog-settings-configuration

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

Microsoft.Extensions.DependencyModel@3.0.0 › Newtonsoft.Json@9.0.1 security issue #331

Open joel-janerup-visma opened 1 year ago

joel-janerup-visma commented 1 year ago

Affected versions of Newtonsoft.Json@9.0.1 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

Fixed in https://www.nuget.org/packages/Microsoft.Extensions.DependencyModel/5.0.0 and new versions

nblumhardt commented 1 year ago

Seems like pushing forward the dependency version here is reasonable - not sure what downstream compat problems this might cause but we've seen some friction caused by targeting 3.0.0 so we might as well bump it up to 6.0.0.

The security issue here won't be reachable via this package (it doesn't itself parse web requests), and code targeting up-to-date web frameworks will either pull in a newer Newtonsoft.JSON or not use it as all, so I think the impact of this is likely close to zero, though it's nice to keep everything fresh 👍

PR welcome, unless any @serilog/maintainers can spot a reason to stay with the current version?

hypdeb commented 1 year ago

One more reason to bump Microsoft.Extensions.DependencyModel is that it has started to cause conflicts with version 7.0.0.

hypdeb commented 1 year ago

I'm unable to push a branch and create a merge request. Here is a modified .csproj file which seems to do the job, at least according to local build and unit tests.

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <Description>Microsoft.Extensions.Configuration (appsettings.json) support for Serilog.</Description>
    <VersionPrefix>3.5.0</VersionPrefix>
    <LangVersion>latest</LangVersion>
    <Authors>Serilog Contributors</Authors>
    <TargetFrameworks>netstandard2.0;net451;net461</TargetFrameworks>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <GenerateDocumentationFile>true</GenerateDocumentationFile>
    <AssemblyName>Serilog.Settings.Configuration</AssemblyName>
    <AssemblyOriginatorKeyFile>../../assets/Serilog.snk</AssemblyOriginatorKeyFile>
    <SignAssembly>true</SignAssembly>
    <PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
    <PackageId>Serilog.Settings.Configuration</PackageId>
    <PackageTags>serilog;json</PackageTags>
    <PackageIcon>icon.png</PackageIcon>
    <PackageProjectUrl>https://github.com/serilog/serilog-settings-configuration/</PackageProjectUrl>
    <PackageLicenseExpression>Apache-2.0</PackageLicenseExpression>
    <RootNamespace>Serilog</RootNamespace>
    <PublishRepositoryUrl>true</PublishRepositoryUrl>
    <EmbedUntrackedSources>true</EmbedUntrackedSources>
    <IncludeSymbols>true</IncludeSymbols>
    <SymbolPackageFormat>snupkg</SymbolPackageFormat>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />
    <PackageReference Include="Serilog" Version="2.10.0" />
    <None Include="..\..\assets\icon.png" Pack="true" PackagePath="" />
  </ItemGroup>

  <ItemGroup Condition="$(TargetFramework) == 'net451'">
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="1.1.2" />
    <PackageReference Include="Microsoft.Extensions.DependencyModel" Version="3.0.0" />
  </ItemGroup>

  <ItemGroup Condition="$(TargetFramework) == 'net461'">
    <PackageReference Include="Microsoft.Extensions.DependencyModel" Version="3.0.0" />
  </ItemGroup>

  <ItemGroup Condition="$(TargetFramework) != 'net451'">
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="2.0.0" />
  </ItemGroup>

  <ItemGroup Condition="$(TargetFramework) != 'net451' And $(TargetFramework) != 'net461'">
    <PackageReference Include="Microsoft.Extensions.DependencyModel" Version="6.0.0" />
  </ItemGroup>

</Project>
skomis-mm commented 1 year ago

Hi @hypdeb ,

Thanks, can you provide more info on what conflicts do you see with .NET 7?

Probably, there are enough reasons to bump MED version. In order to create PR, first fork the repository into your account, push the branch there and then you will be able to create PR.

hypdeb commented 1 year ago

Hello @skomis-mm,

  Microsoft.Common.CurrentVersion.targets(2352, 5): [MSB3277] Found conflicts between different versions of "Microsoft.Extensions.DependencyModel" that could not be resolved.
There was a conflict between "Microsoft.Extensions.DependencyModel, Version=3.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60" and "Microsoft.Extensions.DependencyModel, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60".
    "Microsoft.Extensions.DependencyModel, Version=3.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60" was chosen because it was primary and "Microsoft.Extensions.DependencyModel, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60" was not.
    References which depend on "Microsoft.Extensions.DependencyModel, Version=3.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60" [C:\Users\*****\.nuget\packages\microsoft.extensions.dependencymodel\3.0.0\lib\netstandard2.0\Microsoft.Extensions.DependencyModel.dll].
        C:\Users\*****\.nuget\packages\microsoft.extensions.dependencymodel\3.0.0\lib\netstandard2.0\Microsoft.Extensions.DependencyModel.dll
          Project file item includes which caused reference "C:\Users\*****\.nuget\packages\microsoft.extensions.dependencymodel\3.0.0\lib\netstandard2.0\Microsoft.Extensions.DependencyModel.dll".
            C:\Users\*****\.nuget\packages\microsoft.extensions.dependencymodel\3.0.0\lib\netstandard2.0\Microsoft.Extensions.DependencyModel.dll
    References which depend on "Microsoft.Extensions.DependencyModel, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60" [].
        C:\*****.dll
          Project file item includes which caused reference "C:\*****.dll".
            C:\*****.dll
hypdeb commented 1 year ago

Created a pull request here: https://github.com/serilog/serilog-settings-configuration/pull/339.

I'm not used to changing package versions depending on framework like this, if I'm doing it the wrong way please feel free to tell me. I think for people building against net7.0, using the 7.0.0 version is ideal.

sungam3r commented 1 year ago

Duplicate of #341, fixed in #351.