search

twilio-labs / twilio-aspnet

Integrate Twilio Programmable Messaging and Voice with ASP.NET Respond to webhooks with TwiML in seconds
Apache License 2.0
59 stars 30 forks source link

Remove deprecated Microsoft.AspNetCore.Mvc.Core dependency #70

Closed bcowdery closed 2 years ago

bcowdery commented 2 years ago

The Twilio.AspNet.Core library depends on a long obsolete version of the Microsoft.AspNetCore.Mvc.Core library

Twilio.AspNet.Core@5.68.3
  -> Microsoft.AspNetCore.Mvc.Core >= 1.0.4
        -> Microsoft.AspNetCore.Http >= 1.0.3

This obsolete dependency is vulnerable to CVE-2020-1045, and presents a roadblock to anyone using this package with a project that is under static application security testing (SAST) and other vulnerability detection schemes (like GitHub dependabot).

The nuget.org page for Microsoft.AspNetCore.Mvc.Core@1.0.4 states:

This package has been deprecated as part of the .NET Package Deprecation effort. You can learn more about it from https://github.com/dotnet/announcements/issues/217

bcowdery commented 2 years ago

In my humble opinion, this twilio library should be using MS build conditionals to select the appropriate packages for the target framework. It's odd that the package is still depending on Microsoft.AspNetCore.Mvc.Core at all, let alone a version as old as 1.0.4.

I would suggest targetting Microsoft.AspNetCore.Mvc.Core@2.2.0 for netstandard2.0 and using the framework references for net5.0 and net6.0.

In the current build system, your net6.0 target still depends on Microsoft.AspNetCore.Mvc.Core - this will get flagged as a vulnerable package in most modern build systems, even if it is not used directly. The obsolete package should be conditional only for netstandard build targets so that it doesn't bleed over into the other target frameworks.

suggestion:

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net5.0;net6.0</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">>
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Core" Version="2.2.0" />
    <PackageReference Include="Microsoft.AspNetCore.Http" Version="2.1.22" /> <!-- https://github.com/advisories/GHSA-hxrm-9w7p-39cc -->
  </ItemGroup>

  <ItemGroup Condition="'$(TargetFramework)' == 'net5.0' Or '$(TargetFramework)' == 'net6.0'">>
    <FrameworkReference Include="Microsoft.AspNetCore.App" />
  </ItemGroup>
Swimburger commented 2 years ago

Thank you for reporting this @bcowdery. I agree with your approach and we already have some conditional logic we can extend this with.

Swimburger commented 2 years ago

I'm planning to release this change as part of the major release with breaking changes, but also as minor release 5.77.0. Microsoft.AspNetCore.Mvc.Core 2.2.0 already requires a higher version of Microsoft.AspNetCore.Http.

Swimburger commented 2 years ago

5.77.0 has been released with the updated dependency: https://www.nuget.org/packages/Twilio.AspNet.Core/