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

Remote code execution vulnerability in dependency System.Drawing.Common #6226

Closed
petrikero opened this issue Nov 4, 2022 · 4 comments · Fixed by #6229
Closed

Remote code execution vulnerability in dependency System.Drawing.Common #6226

petrikero opened this issue Nov 4, 2022 · 4 comments · Fixed by #6229
Labels
dependencies Pull requests that update a dependency file security
Milestone

Comments

@petrikero
Copy link
Contributor

Version Information
Version of Akka.NET? 1.4.45 and probably goes back a long way. v1.5 branch also seems affected.
Which Akka.NET Modules? The core Akka module.

Describe the bug
The core module depends on an old System.Configuration.ConfigurationManager version 4.7.0 which transitively depends on System.Common.Drawing v4.7.0. The System.Common.Drawing v4.7.0 is affected by a remote code execution vulnerability GHSA-ghhp-997w-qr28.

To Reproduce
Steps to reproduce the behavior:

  1. akka.net\src\core\Akka>dotnet list package --vulnerable --include-transitive

Expected behavior
No critical vulnerabilities should be found.

Actual behavior
I get the following vulnerabilities (with dev branch on 2022-11-04, commit cebc498):

Project `Akka` has the following vulnerable packages
   [netstandard2.0]:
   Top-level Package      Requested   Resolved   Severity   Advisory URL
   > Newtonsoft.Json      [12.0.3,)   12.0.3     High       https://github.com/advisories/GHSA-5crp-9r3c-p9vr

   [net6.0]:
   Top-level Package      Requested   Resolved   Severity   Advisory URL
   > Newtonsoft.Json      [12.0.3,)   12.0.3     High       https://github.com/advisories/GHSA-5crp-9r3c-p9vr

   Transitive Package           Resolved   Severity   Advisory URL
   > System.Drawing.Common      4.7.0      Critical   https://github.com/advisories/GHSA-rxg9-xrhp-64gj

Additional context
The fix should be pretty easy, just upgrade the vulnerable dependencies to more recent versions.

I think it would be nice to check for the vulnerable packages in the CI jobs for Akka with something like:

# note that the dotnet executable returns an exit code 0 even when there are vulnerabilities so you need to grep the output explicitly
dotnet list package --vulnerable --include-transitive | tee vulnerable.out
test `grep -cm 1 'has the following vulnerable packages' vulnerable.out` = 0

The Newtonsoft.Json dependency is also rather old (from 2019) and has a vulnerability, so should probably also be updated.

@Aaronontheweb
Copy link
Member

Weird, we haven't been getting dependabot alerts for any of these. I wonder if we're running into issues with the max number of outstanding PRs for it.

@Aaronontheweb Aaronontheweb added dependencies Pull requests that update a dependency file security labels Nov 4, 2022
Aaronontheweb added a commit that referenced this issue Nov 4, 2022
#6226 <-- ran into this issue here.
@Aaronontheweb
Copy link
Member

Yep, that was it: #6227

@Aaronontheweb
Copy link
Member

Once that's pulled in we should get some CI updates for this. I agree that it'd be a good idea for us to add either CodeQL or some other security scanning service - discussed this over the summer with one of our customers who works in the space.

Aaronontheweb added a commit that referenced this issue Nov 4, 2022
#6226 <-- ran into this issue here.
Aaronontheweb added a commit that referenced this issue Nov 5, 2022
* Upgrade to System.Configuration.ConfigurationManager 6.0.1

close #6226

* removed second dependency on System.Configuration.ConfigurationManager

* fixed references
@Aaronontheweb Aaronontheweb added this to the 1.4.46 milestone Nov 15, 2022
@Aaronontheweb
Copy link
Member

FYI, following the resolution of this issue we've added CodeQL vulnerability scanning to all PRs going forward #6254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants