search

Roave / SecurityAdvisoriesBuilder

:hammer: Build tools responsible for assembling https://github.com/Roave/SecurityAdvisories/blob/master/composer.json
MIT License
10 stars 7 forks source link

Include advisory source in new security advisory commits #451

Open Ocramius opened 2 years ago

Ocramius commented 2 years ago

One regular source of support questions is "why is some/library:1.2.3 included in the roave/security-advisories conflicts section?"

This is becoming regular and quite frustrating:

We probably do want to start committing the source of an advisory.

Specifically, we need to add a Source (value object with a URI in it, basically) to Advisory:

https://github.com/Roave/SecurityAdvisoriesBuilder/blob/02469333eaacc55a0e621f534e1fcf88e2608979/src/Roave/SecurityAdvisories/Advisory.php#L33-L46

After doing that comes the tricky part: we need to identify which advisories were not considered as part of the pre-existing composer.json.

For that, we need to:

  1. read the pre-existing composer.json into an usable in-memory data structure
  2. compare each of the Advisory instances against it
  3. isolate those Advisory instances that would lead to a change of the excluded (data structure above?)
  4. add these Advisory instances to a list that is then used to determine the commit message to be generated
  5. generate the new commit message, which is currently hardcoded: https://github.com/Roave/SecurityAdvisoriesBuilder/blob/14a83da88ab7a0fe80a08a8719f84ea12bf0d7c3/build-conflicts.php#L247-L275
slash3b commented 2 years ago

Locally, I've just deleted some of packages from the "old" version and do comparison of newly generated composer.json to the old one. So this way we will have a a list of packages that are not in the old composer.json version.

Please check example below. Is this what we need ?

/usr/bin/php /home/slash3b/Projects/php/SecurityAdvisoriesBuilder/build-conflicts.php
Committing generated "composer.json" file as per "2021-11-20T22:33:36+00:00"
Original commit: "https://github.com/FriendsOfPHP/security-advisories/commit/486a92e290b38e5f01f0075c3919a5d251960671"

Added advisories:
   Package name: "alterphp/easyadmin-extension-bundle"
   Summary: "Action case insensitivity"
   URI: "https://github.com/alterphp/EasyAdminExtensionBundle/releases/tag/v1.3.1"

   Package name: "3f/pygmentize"
   Summary: "Remote Code Execution"
   URI: "https://github.com/dedalozzo/pygmentize/issues/1"

   Package name: "adodb/adodb-php"
   Summary: "XSS vulnerability in old test script"
   URI: "https://github.com/ADOdb/ADOdb/issues/274"

   Package name: "adodb/adodb-php"
   Summary: "Potential SQL injection vector"
   URI: "https://github.com/ADOdb/ADOdb/pull/401"

   Package name: "akaunting/akaunting"
   Summary: "Malicious password-reset in Akaunting"
   URI: "https://github.com/advisories/GHSA-246r-r2wf-frhx"
Ocramius commented 2 years ago

That seems exactly like what we're trying to achieve here, yes 👍