Scan for vulnerable NodeJS and Composer packages

[chillu]

Overview

There's been a couple of high profile compromises of downstream dependencies in the NodeJS ecosystem. We should build security checks for this into our CI process, and fail the build to surface this early. See https://blog.risingstack.com/controlling-node-js-security-risk-npm-dependencies/

Acceptance Criteria

• Runs on every pull request, preventing a bump to an insecure version
• Runs nightly so we can detect insecure dependencies even on modules without a lot of commit or pull request activity
• Can inspect all actively maintained release branches (not just the default branch)
• Fails builds on detecting insecure deps
• Notifies product teams on failures
• Allows for centralised reporting (e.g. for infosec compliance)
• Works with composer packages
• Works with NodeJS packages (where present in modules)
• Applied to all supported modules

Notes

• Related: Include security-advisories in composer.json
• Ideally we're choosing a solution that can also be adopted for private repos (everyone's customers) without breaking the bank, although that's not a blocker
• Centralised reporting might be achieved by pushing data into our SS Ltd data warehouse, and sending reports from there

Options

NPM Audit (Node)

• https://blog.npmjs.org/post/173719309445/npm-audit-identify-and-fix-insecure
• Official tool by package manager
• Requires NPM v6, which I think requires Node v10, which I think we're using already :D
• Presumably what Github already does with it's security alerts, but we can also apply this to non-Github repos

Snyk (PHP + Node)

• https://snyk.io/
• Free for OSS, $2k+ for on premise Gitlab
• Great Github integration for static anaylsis
• Good reporting aggregation
• Can review license compliance
• Doesn't have any vulns for SilverStripe, so doesn't use CVE or Symfony Security Database: https://snyk.io/vuln/search?q=silverstripe&type=composer, even when searching for a specific known SilverStripe CVE: https://snyk.io/vuln/search?q=CVE-2017-18049&type=composer
• Seems to require specific vuln reporting to their databse (the api doesn't have an endpoint for automating this)

Tidelift (PHP + Node)

• https://tidelift.com
• They incentivise well maintained packages by paying open source maintainers who sign up and agree to their standards, see https://tidelift.com/docs/lifting/
• Looks like they have their own vuln database you have to report to: https://tidelift.com/docs/lifting/security

Symfony Security Monitoring (PHP)

• https://security.symfony.com/
• From 2EUR/project/month prepaid for three years. I've contacted them two months ago about exceptions for large open source projects, but haven't heard back. We could pay this for the CWP kitchen sink, which covers all of our supported modules - and give back to the community.
• It's unclear how this is superior to Travis nightly builds (with email build failure notifications) using the sensiolabs/security-checker which are presumably using the same source data and the same web service. The service states "Manual checking is free" and has a disclaimer saying the service is provided as-is.
• Uses the Symfony Security Advisories Database

Github Security Alerts (Node)

• See announcement.
• Not really an option because it doesn't understand PHP dependencies - yet.
• Uses CVE, but it's unclear how they relate the arbitrary CVE vendor and product denominators to specific dependencies (e.g. composer package names) - see CVE usage details
• Can't scan repos outside of Github, so couldn't cover all of our projects

GuardRails (Node + PHP)

• Wider service for static code analysis
• See https://docs.guardrails.io/vulnerabilities/php/using_vulnerable_libraries.html
• Uses symfony/security-checker and NPM audit under the hood: https://docs.guardrails.io/tools.html

[blueo]

this is possibly an option: https://snyk.io/ can scan php dependencies too.

[chillu]

Synk runs their own PHP Vulnerability Database so we'd need to report issues in our own modules there, as well as encouraging the community to do so. The Symfony Security project seems more likely to get traction?

Symfony costs money (at least 2EUR per month), which is generally fine in terms of supporting friendly open source colleagues. But the "per project" bit is a bit worrying: We have a LOT of projects to check (70+ supported modules). I've contacted them with the following message:

---

Hello!

We're running an open source PHP CMS (silverstripe.org) with dozens of modules supported by a dedicated team (https://www.silverstripe.org/software/addons/silverstripe-commercially-supported-module-list/). Incidentally we're using a lot of Symfony components, so thanks for creating awesome software!

We'd like to tighten up our security practices around secure dependencies, so looking to integrate Symfony Security through API checks in our Travis-based CI. We're generally happy to pay a small amount towards this, since we're running a business on top of these open source offerings.

The tricky part is what you consider a "project". We have quite a lot of modules, each with their own CI builds. Do you interpret "project" as "repo"? In which case your service would likely be out of our price range. Or a more loose definition of "project" where multiple modules could upload differing composer.lock files?

Thanks, and all the best from Wellington, New Zealand Ingo

[maxime-rainville]

Silly suggestion, but GitHub seems to be pretty good at notifying us when we're using vulnerable node dependencies. Is there a problem with relying on that?

[chillu]

Github only covers NodeJS dependencies for us, it doesn't scan PHP. Ideally we have one solution which does both. Even if it did scan PHP, we've got repos outside of Github (e.g. in CWP Gitlab). I'm aiming for the least amount of solutions covering the most surface here. Might not be practical though.

FYI, I haven't heard back from Symfony Security.

[chillu]

Of course I missed one: https://dependabot.com/ has been acquired by Github, and does this all for free. Including PHP! I can't see where it gets security advisories from in the case of PHP (since they're not provided by composer/packagist directly)

[chillu]

It appears that the "auto-merge" feature is optional (as it should be), but Dependabot always asks for write permissions to code in the repo. That's not cool, I don't want it to have that level of access.

I've contacted them about the possibility to set more granular permissions

[chillu]

Answer from Dependabot:

[chillu]

Github has Composer/PHP support for this now! https://github.blog/2019-09-18-dependency-graph-supports-php-repos-with-composer-dependencies/

[brynwhyman]

Just a note on Dependabot - it creates branches on our repositories, which will show up in places like Packagist. Not the end of the world, but it's something we currently look to avoid with things like community pull requests (which should come from a fork).

[brynwhyman]

Noting that it looks like GitHub is rolling out Dependabot to all repositories by default anyway, we're seeing them pop up on our commercially supported modules already.

Given this is being rolled out more widely, having Dependabot creating branches on origin might not be that big of a deal? After a merge they're deleted automatically anyway.

See https://help.github.com/en/github/managing-security-vulnerabilities/configuring-automated-security-fixes:

We'll automatically enable automated security fixes in every repository that uses security alerts and the dependency graph over the next few months, starting in May 2019.

[ScopeyNZ]

You can still opt out. Annoying that they're enabling by default though. Most cases they can be merged without worrying, but in some cases dist files need to be rebuilt (implying that the security issue actually matters - it doesn't if dist files don't need changing) meaning that the PR is a bit useless.

In summary, they are pointless and can be ignored if the dist files aren't affected. They should be done manually if they require rebuilt dist files. I still think we should opt out for now.

[chillu]

In summary, they are pointless and can be ignored if the dist files aren't affected. They should be done manually if they require rebuilt dist files. I still think we should opt out for now.

Agreed. The important part is the triage ("does this issue affect our users?"), and tracking issues. I think unless we deem it a high/critical dependency issue, we can just deal with this by a regular sweep across the repos. That seems more viable than scattering everyone's attention by checking out PRs, rebuilding dist, pushing, waiting for build to pass, merging - every time a dependency issue comes up.

Maybe there's a way that we could auto-fix those dependabot PRs through Github Actions or something? Might be more hassle than it's worth for now to build this of course.

[robbieaverill]

I also wondered whether you can opt out of this new feature in bulk across the organisation - that'd be nice

[chillu]

Argh, I've just added one criteria which throws off our plans to use Github's built in advisories:

• Can inspect all actively maintained release branches (not just the default branch)

According to https://github.community/t5/How-to-use-Git-and-GitHub/Is-it-possible-to-get-security-updates-for-a-branch-other-than/m-p/21676, that's not possible. So in the case of silverstripe/framework, we'd get alerts for 4.x, but not 3.x.

[chillu]

Also, it looks like just because a vulnerability has been reported as a CVE, even though Github claims to use that as a source, there's cases where it doesn't come through (see forum post). That's a Java/POM dependency, maybe it's more clear cut with PHP/Composer.

[chillu]

Update on this: Snyk has greatly improved their PHP support, see "silverstripe" search, php instructions and silverstripe/framework overview

[chillu]

Update from Github support:

We've removed FriendsOfPHP as a listed source for GitHub Advisory Database until we're able to do a full import of FriendsOfPHP data.