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

Add support for automatically disclosing security issues #85

Open
robbieaverill opened this issue May 23, 2018 · 7 comments
Open

Add support for automatically disclosing security issues #85

robbieaverill opened this issue May 23, 2018 · 7 comments

Comments

@robbieaverill
Copy link

robbieaverill commented May 23, 2018

The SensioLabs security checker (and the PHP package for it) uses https://github.com/FriendsOfPHP/security-advisories as a data source, which contains some of our older vulnerabilities that were manually added.

It would be nice for cow to automate this process to some extent, so we can incorporate pushing out security notifications to repositories like this one. This would add weight to some of the features of CWP coming up like the "site summariser" module which includes listing known security vulnerabilities in the report: https://github.com/bringyourownideas/silverstripe-composer-security-checker and https://github.com/bringyourownideas/silverstripe-maintenance/

Some implementation ideas:

  • Cow could have an interactive command for the user to input the vulnerability data for anything detected in the changelog during a release - this could be pre-filled with the parts that cow knows about (the SS identifier, name, website URL)
  • silverstripe.org could have a feature added to export a YAML file for each of the security vulnerability pages (which could then be copy/pasted into a pull request manually)
  • We could change silverstripe.org to use this as a data source as well, dogfooding it ourselves

Other considerations:

  • If we use silverstripe.org to power this we will probably need to add more structured data fields, since the "versions affected" field doesn't distinguish repositories well (for example)
  • A fully automated process in cow would require the hub CLI for github or something similar installed in order to create the pull request
  • Check to see whether there's a schema and/or a validator for said schema for this repository (edit: there is a validator for the repository as a whole)

Contributing guide: https://github.com/FriendsOfPHP/security-advisories#contributing

First priority would be to ensure that this repository is up to date with the current list of disclosed vulnerabilities (cc @stevie-mayhew)

@robbieaverill
Copy link
Author

robbieaverill commented Jul 16, 2018

If we use silverstripe.org to power this we will probably need to add more structured data fields, since the "versions affected" field doesn't distinguish repositories well (for example)

Just thought about this again. This is what we have in the SilverStripe.org CMS at the moment:

image

We could probably replace this with two textareas and use a structure like one line per module, and the format of [composer module name]: [semver constraint] for each line. This would be structured enough and would require minimal changes to the website (replace new lines with comma delimited). Edit: actually we can probably leave this as it is and just ensure we have consistent structure in the composer module name and semver constraint. Would require a little retrospective editing of ss.org advisories to ensure they all conform.

We could potentially also add an API endpoint to expose the structured data for all security announcements on the website as structured JSON, and then this command could essentially run a sync command to pull the security advisory repository, rebuild all security alerts and make a commit if anything new is added.

@chillu do you have any thoughts on this? I'll probably pick it up on hackday.

@ScopeyNZ
Copy link

@ScopeyNZ
Copy link

Also, a team is overhauling SS.org right now - I'm trying to get some more defined form fields for adding security advisories in the CMS.

@robbieaverill
Copy link
Author

robbieaverill commented Nov 29, 2018

😄

A multi-input list of composer package names and semver constraint fields would be ideal, e.g.:

Affected package Constraint Fix version(s)
silverstripe/framework ^4.0.2 4.0.9, 4.1.1, 4.4.3
silverstripe/admin ^4.0 <4.3 4.0.2, 4.1.1, 4.2.9

Just an idea...

@chillu
Copy link
Member

chillu commented Nov 26, 2019

I think we should base this on Github's new security advisories, rather than FriendsOfPHP. Which submits CVEs as well. Between those two, we cover more ground than through FriendsOfPHP + CVE? See silverstripe/silverstripe-framework#9335

@robbieaverill
Copy link
Author

It'd be nice if GitHub exposes the ability to do this via their API at some point!

@chillu
Copy link
Member

chillu commented Feb 2, 2020

Here's an example of the confusion caused by not being clear about the module that a release relates to :)

A multi-input list of composer package names and semver constraint fields would be ideal, e.g.:

You could also argue that if a vuln is present in more than one module, it should be flagged as multiple vulns. That makes it less ambiguous if you're still running vulnerable code as well. https://github.com/FriendsOfPHP/security-advisories/ doesn't seem to allow referencing multiple packages in a single issue, so we shouldn't either.

I'd argue that there's two levels of information:

  1. Advisory: Should reference exactly one module, and one or more version constraints. Should have a CVE, and a description. It should only reference a recipe if the vuln is actually in the code provided of that recipe, not in its dependencies.
  2. Recipe release: A recipe is made up of multiple modules. The release notes should reference any advisory contained in that release and its dependencies.

Proposed ACs:

  • There's one source of truth for advisories published by Silverstripe
  • Duplication of advisory information into other advisory databases should be kept to a minimum, referencing the source of truth
  • Publication of an advisory is under control of Silverstripe as part of the release process (the release isn't blocked by third parties)
  • Every advisory relates to a single module
  • We have a high degree of certainty that advisory drafts are published as part of a recipe release, and referenced in the release notes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants