From 78f93c74a8c02779628aec77124823b81dae2d4f Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Wed, 5 May 2021 22:37:54 +0100 Subject: [PATCH 1/4] Move some logic out of the command --- src/AdvisoriesManager.php | 83 +++++++++++++++++++++++++++++++++++++-- src/AuditCommand.php | 28 ++----------- 2 files changed, 83 insertions(+), 28 deletions(-) diff --git a/src/AdvisoriesManager.php b/src/AdvisoriesManager.php index d581b28..7bc4c56 100644 --- a/src/AdvisoriesManager.php +++ b/src/AdvisoriesManager.php @@ -4,6 +4,11 @@ use Composer\Composer; use Composer\Plugin\PluginInterface; +use Composer\Semver\Constraint\Constraint; +use Composer\Semver\Constraint\ConstraintInterface; +use Composer\Semver\Constraint\MultiConstraint; +use Composer\Semver\VersionParser; +use Symfony\Component\Yaml\Yaml; /** * Handles finding security advisories. @@ -19,15 +24,22 @@ final class AdvisoriesManager /** @var AdvisoriesInstaller */ private $installer; + /** @var VersionParser */ + private $versionParser; + private $packageName = 'sensiolabs/security-advisories'; private $packageConstraint = 'dev-master'; /** @var string */ private $directory; + /** @var array> */ + private $advisories; + public function __construct(AdvisoriesInstaller $installer) { $this->installer = $installer; + $this->versionParser = new VersionParser(); } public static function create(Composer $composer) @@ -53,12 +65,75 @@ public function mustUpdate() $this->installer->mustUpdate(); } - public function findAll() + /** + * Find all advisories. + * + * @return iterable> + */ + public function findAll(): iterable { - $advisoriesDir = $this->getDirectory(); + if (!isset($this->advisories)) { + $this->advisories = []; + + $advisoriesDir = $this->getDirectory(); + \assert(is_dir($advisoriesDir)); + + // Find all the advisories for installed packages. + foreach (glob("$advisoriesDir/*/*/*.yaml") as $file) { + $advisory = Yaml::parseFile($file); + + $this->advisories[$file] = $advisory; + } + } + + yield from $this->advisories; + } + + /** + * Find any advisory applying to the given package name and version. + * + * @return iterable> + */ + public function findByPackageNameAndVersion(string $name, string $version): iterable + { + $reference = sprintf('composer://%s', $name); + $constraint = new Constraint('==', $this->versionParser->normalize($version)); + + foreach ($this->findAll() as $advisory) { + if ($advisory['reference'] === $reference) { + if ($this->createConstraint($advisory)->matches($constraint)) { + yield $advisory; + } + } + } + } + + /** + * Construct contstraint from the advistory. + */ + private function createConstraint(array $advisory): ConstraintInterface + { + $constraints = []; + + foreach ($advisory['branches'] as $branch) { + $branchConstraints = []; + + foreach ($branch['versions'] as $version) { + $branchConstraints[] = $this->versionParser->parseConstraints($version); + } + + if ($branchConstraints !== []) { + $constraints[] = count($branchConstraints) > 1 + ? new MultiConstraint($branchConstraints, true) + : $branchConstraints[0]; + } + } + + if (\count($constraints) === 1) { + return $constraints[0]; + } - // Find all the advisories for installed packages. - return glob("$advisoriesDir/*/*/*.yaml"); + return new MultiConstraint($constraints, false); } private function getDirectory() diff --git a/src/AuditCommand.php b/src/AuditCommand.php index 870e6ed..d413a7c 100644 --- a/src/AuditCommand.php +++ b/src/AuditCommand.php @@ -100,34 +100,14 @@ protected function execute(InputInterface $input, OutputInterface $output) $packages = $lockData['packages']; } - $packages = array_map(static function (array $package): array { - return [ - 'name' => $package['name'], - 'version' => $package['version'], - 'reference' => sprintf('composer://%s', $package['name']), - ]; - }, $packages); - - $packages = array_column($packages, 'version', 'reference'); + $packages = array_column($packages, 'version', 'name'); $advisories = []; // Find all the advisories for installed packages. - foreach ($advisoriesManager->findAll() as $file) { - $advisory = Yaml::parseFile($file); - $advisory['_file'] = $file; - - if (isset($packages[$advisory['reference']])) { - $installedVersion = $packages[$advisory['reference']]; - - foreach ($advisory['branches'] as $branch) { - $constraint = implode(',', $branch['versions']); - - if (Semver::satisfies($installedVersion, $constraint)) { - $advisories[$advisory['reference']][] = $advisory; - break; - } - } + foreach ($packages as $name => $version) { + foreach ($advisoriesManager->findByPackageNameAndVersion($name, $version) as $advisory) { + $advisories[$name][] = $advisory; } } From d230b2f59d3d864b5aa52f20aa2665610ee6ac40 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 6 May 2021 00:07:44 +0100 Subject: [PATCH 2/4] Create interface of advisories installer --- src/AdvisoriesInstaller.php | 2 +- src/AdvisoriesInstallerInterface.php | 33 ++++++++++++++++++++++++++++ src/AdvisoriesManager.php | 4 ++-- 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 src/AdvisoriesInstallerInterface.php diff --git a/src/AdvisoriesInstaller.php b/src/AdvisoriesInstaller.php index a11f8da..2d8ed77 100644 --- a/src/AdvisoriesInstaller.php +++ b/src/AdvisoriesInstaller.php @@ -15,7 +15,7 @@ * @copyright 2020 Chris Smith * @license MIT */ -abstract class AdvisoriesInstaller +abstract class AdvisoriesInstaller implements AdvisoriesInstallerInterface { /** @var RepositoryManager */ private $repositoryManager; diff --git a/src/AdvisoriesInstallerInterface.php b/src/AdvisoriesInstallerInterface.php new file mode 100644 index 0000000..77c97fd --- /dev/null +++ b/src/AdvisoriesInstallerInterface.php @@ -0,0 +1,33 @@ +> */ private $advisories; - public function __construct(AdvisoriesInstaller $installer) + public function __construct(AdvisoriesInstallerInterface $installer) { $this->installer = $installer; $this->versionParser = new VersionParser(); From bb6744d55e0d70818f57ab15b4977c82083f3a90 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Fri, 14 May 2021 09:28:27 +0100 Subject: [PATCH 3/4] Adjust output to match existing output --- src/AuditCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AuditCommand.php b/src/AuditCommand.php index d413a7c..7fd172b 100644 --- a/src/AuditCommand.php +++ b/src/AuditCommand.php @@ -138,7 +138,7 @@ protected function execute(InputInterface $input, OutputInterface $output) ksort($advisories, \SORT_NATURAL | \SORT_ASC); foreach ($advisories as $reference => $packageAdvisories) { - $output->writeln(sprintf('%s (%s)', $reference, $packages[$reference])); + $output->writeln(sprintf('composer://%s (%s)', $reference, $packages[$reference])); foreach ($packageAdvisories as $advisory) { $title = $advisory['title']; From 0a2d435f7092200fc61a08c7d296dfb23f639700 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Fri, 14 May 2021 09:28:51 +0100 Subject: [PATCH 4/4] Add initial unit tests --- phpunit.xml | 7 ++ tests/unit/AdvisoriesManagerTest.php | 91 +++++++++++++++++++ tests/unit/advisories/empty/composer.json | 6 ++ tests/unit/advisories/simple/composer.json | 6 ++ .../unit/advisories/simple/foo/bar/vuln1.yaml | 8 ++ 5 files changed, 118 insertions(+) create mode 100644 tests/unit/AdvisoriesManagerTest.php create mode 100644 tests/unit/advisories/empty/composer.json create mode 100644 tests/unit/advisories/simple/composer.json create mode 100644 tests/unit/advisories/simple/foo/bar/vuln1.yaml diff --git a/phpunit.xml b/phpunit.xml index 696252c..8bada5f 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -9,6 +9,9 @@ beStrictAboutTodoAnnotatedTests="true" > + + tests/unit + tests/integration @@ -19,4 +22,8 @@ src + + + + diff --git a/tests/unit/AdvisoriesManagerTest.php b/tests/unit/AdvisoriesManagerTest.php new file mode 100644 index 0000000..0e69dcd --- /dev/null +++ b/tests/unit/AdvisoriesManagerTest.php @@ -0,0 +1,91 @@ +createManager($advisories); + $results = []; + + foreach ($manager->findByPackageNameAndVersion($packageName, $packageVersion) as $advisory) { + $results[] = $advisory['title']; + + self::assertEquals(sprintf('composer://%s', $packageName), $advisory['reference']); + } + + self::assertEquals($expected, $results); + } + + public function dataFindByPackageNameAndVersion(): iterable + { + yield [ + [], + 'foo/bar', + '13.37.0', + 'empty', + ]; + yield [ + [ + 'CVE-9999-1234567: Left the front door open', + ], + 'foo/bar', + '13.37', + 'simple', + ]; + } + + private function createManager(string $advisories): AdvisoriesManager + { + $installer = new class($advisories) implements AdvisoriesInstallerInterface { + private $advisories; + + public function __construct(string $advisories) + { + $this->advisories = __DIR__.'/advisories/'.$advisories; + + if (!is_dir($this->advisories)) { + throw new \InvalidArgumentException(sprintf( + '%s is invalid, `%s` is not a directory', + $advisories, + $this->advisories + )); + } + } + + public function mustUpdate() + { + return; // No op + } + + public function install($varDirectory, $packageName, $packageConstraint) + { + return $this->advisories; + } + }; + + return new AdvisoriesManager($installer); + } +} diff --git a/tests/unit/advisories/empty/composer.json b/tests/unit/advisories/empty/composer.json new file mode 100644 index 0000000..78bfed1 --- /dev/null +++ b/tests/unit/advisories/empty/composer.json @@ -0,0 +1,6 @@ + +{ + "name": "sensiolabs/security-advisories", + "description": "Database of known security vulnerabilities in various PHP projects and libraries", + "license": "Unlicense" +} diff --git a/tests/unit/advisories/simple/composer.json b/tests/unit/advisories/simple/composer.json new file mode 100644 index 0000000..78bfed1 --- /dev/null +++ b/tests/unit/advisories/simple/composer.json @@ -0,0 +1,6 @@ + +{ + "name": "sensiolabs/security-advisories", + "description": "Database of known security vulnerabilities in various PHP projects and libraries", + "license": "Unlicense" +} diff --git a/tests/unit/advisories/simple/foo/bar/vuln1.yaml b/tests/unit/advisories/simple/foo/bar/vuln1.yaml new file mode 100644 index 0000000..432fbb7 --- /dev/null +++ b/tests/unit/advisories/simple/foo/bar/vuln1.yaml @@ -0,0 +1,8 @@ +title: "CVE-9999-1234567: Left the front door open" +link: https://example.com/CVE-9999-1234567 +cve: CVE-9999-1234567 +branches: + "1337": + time: 2020-01-01 12:32:00 + versions: ['>=13.37.0', '<13.37.100'] +reference: composer://foo/bar