From 07c192163158d2d21c6d6a7e1a0d7228dd690868 Mon Sep 17 00:00:00 2001 From: Ryan Hoerr Date: Fri, 1 Mar 2019 22:11:22 -0500 Subject: [PATCH 1/2] Refactored MageRun plugin; added frontname route checking for unknown modules. --- .../Magerun/SecurityScanCommand.php | 222 ++++++++++++++---- 1 file changed, 178 insertions(+), 44 deletions(-) diff --git a/src/ModuleBlacklist/Magerun/SecurityScanCommand.php b/src/ModuleBlacklist/Magerun/SecurityScanCommand.php index da42e59..457d8a9 100644 --- a/src/ModuleBlacklist/Magerun/SecurityScanCommand.php +++ b/src/ModuleBlacklist/Magerun/SecurityScanCommand.php @@ -18,7 +18,12 @@ class SecurityScanCommand extends AbstractMagentoCommand { const BLACKLIST_URL = 'https://raw.githubusercontent.com/gwillem/magevulndb/master/magento1-vulnerable-extensions.csv'; - + + /** + * @var array + */ + private $routeMap; + /** * @return void */ @@ -28,70 +33,199 @@ protected function configure() ->setDescription('Check installed modules for known vulnerabilities'); } - /** - * @param \Symfony\Component\Console\Input\InputInterface $input - * @param \Symfony\Component\Console\Output\OutputInterface $output - * @return int exit code: 0 no known vulnerabilities found, 1 vulnerabilities found, 2 data could not be loaded - */ + /** + * Check the current Magento install for any modules matching MageVulnDb. + * + * @param \Symfony\Component\Console\Input\InputInterface $input + * @param \Symfony\Component\Console\Output\OutputInterface $output + * @return int exit code: 0 no known vulnerabilities found, 1 vulnerabilities found, 2 data could not be loaded + */ protected function execute(InputInterface $input, OutputInterface $output) { $this->detectMagento($output); if ($this->initMagento()) { - $modules = \Mage::getConfig()->getNode()->modules; - $blacklist = fopen(static::BLACKLIST_URL, 'r'); - $exitCode = 0; + $blacklist = fopen(static::BLACKLIST_URL, 'rb'); $hitCount = 0; - + if ($blacklist === false) { $output->writeln( 'Unable to load the latest vulnerability data.', OutputInterface::VERBOSITY_QUIET ); + return 2; } - - while ($row = fgetcsv($blacklist)) { - $name = $row[0]; - $fixedIn = $row[1]; - $credit = $row[3]; - $updateUrl = $row[4]; - $version = isset($modules->{$name}->version) ? $modules->{$name}->version : null; - - if ($version !== null - && (empty($fixedIn) || version_compare($modules->{$name}->version, $fixedIn, '<'))) { - $output->writeln( - sprintf( - 'Vulnerable module found: %s%s', - $name, - $output->isQuiet() && !empty($fixedIn) ? sprintf(' (%s < %s)', $version, $fixedIn) : '' - ), - OutputInterface::VERBOSITY_QUIET - ); - - $output->writeln(sprintf('Installed: %s', $modules->{$name}->version)); - $output->writeln(sprintf('Fixed In: %s', $fixedIn ?: '(none)')); - - if (!empty($updateUrl)) { - $output->writeln(sprintf('Update URL: %s', $updateUrl)); - } - - if (!empty($credit)) { - $output->writeln(sprintf('Credit: %s', $credit)); - } - - $output->writeln(''); - - $exitCode = 1; + + while ($row = $this->getRowObject(fgetcsv($blacklist))) { + if ($this->checkIsInstalledModule($output, $row) || $this->checkIsInstalledRoute($output, $row)) { $hitCount++; } } if ($hitCount === 0) { $output->writeln('No known vulnerable modules detected.'); + return 0; } - return $exitCode; + return 1; + } + + return 2; + } + + /** + * Check row for match in installed modules, by version + * + * @param \Symfony\Component\Console\Output\OutputInterface $output + * @param \Varien_Object $row + * @return int + */ + protected function checkIsInstalledModule(OutputInterface $output, \Varien_Object $row) + { + // No match if module has no installed version, or version is equal/greater than fixed-in. + if ($row->getVersion() === null + || (!empty($row->getFixedIn()) && version_compare($row->getVersion(), $row->getFixedIn(), '>='))) { + return false; + } + + $output->writeln( + sprintf( + 'Vulnerable module found: %s%s', + $row->getName(), + $output->isQuiet() && !empty($row->getFixedIn()) + ? sprintf(' (%s < %s)', $row->getVersion(), $row->getFixedIn()) + : '' + ), + OutputInterface::VERBOSITY_QUIET + ); + + $output->writeln(sprintf('Installed: %s', $row->getVersion())); + $output->writeln(sprintf('Fixed In: %s', $row->getFixedIn() ?: '(unknown)')); + + if (!empty($row->getUpdateUrl())) { + $output->writeln(sprintf('Update URL: %s', $row->getUpdateUrl())); + } + + if (!empty($row->getCredit())) { + $output->writeln(sprintf('Credit: %s', $row->getCredit())); + } + + $output->writeln(''); + + return true; + } + + /** + * Check for match in frontend routes if module is unknown + * + * @param \Symfony\Component\Console\Output\OutputInterface $output + * @param \Varien_Object $row + * @return bool + */ + protected function checkIsInstalledRoute(OutputInterface $output, \Varien_Object $row) + { + $module = $this->getModuleByRoute($row->getFrontname()); + + if ($module === null || ($row->getName() !== '?' && !empty($row->getName()))) { + return false; } + + $output->writeln( + sprintf( + 'Potential vulnerable module found: %s%s', + $module, + $output->isQuiet() ? sprintf(' (route match: %s)', $row->getFrontname()) : '' + ), + OutputInterface::VERBOSITY_QUIET + ); + + $output->writeln( + '' + . 'Matched by route: This may be a false positive where your installed module' . "\n" + . 'shares it with a vulnerable module, but it should be investigated further.' . "\n" + . 'Please contribute info about the module to MageVulnDb if it is relevant.' + . '' + ); + + $output->writeln(sprintf('Route: %s', $row->getFrontname())); + $output->writeln(sprintf('Looks Like: %s', $row->getRoute())); + $output->writeln(sprintf('Module: %s', $module)); + $output->writeln(sprintf('Installed: %s', $this->getModuleVersion($module))); + + if (!empty($row->getUpdateUrl())) { + $output->writeln(sprintf('Update URL: %s', $row->getUpdateUrl())); + } + + if (!empty($row->getCredit())) { + $output->writeln(sprintf('Credit: %s', $row->getCredit())); + } + + $output->writeln(''); + + return true; + } + + /** + * Get the module tag of the installed module matching the given route (if any). + * + * @param string $frontName + * @return string|null + */ + protected function getModuleByRoute($frontName) + { + if ($this->routeMap === null) { + $routers = \Mage::getConfig()->getNode()->frontend->routers->asArray(); + $this->routeMap = array(); + foreach ($routers as $router) { + if (isset($router['args']['frontName'], $router['args']['module'])) { + $this->routeMap[strtolower($router['args']['frontName'])] = $router['args']['module']; + } + } + } + + $frontName = strtolower($frontName); + + return isset($this->routeMap[$frontName]) + ? $this->routeMap[$frontName] + : null; + } + + /** + * Get the installed version of the given module tag (if any). + * + * @param string $moduleTag + * @return \SimpleXMLElement|null + */ + protected function getModuleVersion($moduleTag) + { + return isset(\Mage::getConfig()->getNode()->modules->{$moduleTag}->version) + ? \Mage::getConfig()->getNode()->modules->{$moduleTag}->version + : null; + } + + /** + * Turn a MageVulnDb M1 CSV row into a keyed Varien_Object. + * + * @param array $csvRow + * @return \Varien_Object|false + */ + protected function getRowObject($csvRow) + { + if (!is_array($csvRow)) { + return false; + } + + $route = trim($csvRow[2], '/'); + + return new \Varien_Object(array( + 'name' => $csvRow[0], + 'version' => $this->getModuleVersion($csvRow[0]), + 'fixed_in' => $csvRow[1], + 'route' => $route, + 'frontname' => strpos($route, '/') ? substr($route, 0, strpos($route, '/')) : $route, + 'credit' => $csvRow[3], + 'update_url' => $csvRow[4], + )); } } From 4c8650eb9666e6305daa19fbcbb5d27fb7134404 Mon Sep 17 00:00:00 2001 From: Ryan Hoerr Date: Tue, 5 Mar 2019 21:35:17 -0500 Subject: [PATCH 2/2] Improved frontname parsing and updated readme, per review. (#23) --- README.md | 8 ++++-- .../Magerun/SecurityScanCommand.php | 28 ++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 9d0615d..4d097c8 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,10 @@ We register the newest only and advice everybody to upgrade to the latest versio The name as registered in the code (and output by `n98-magerun dev:module:list`) is leading. If a module is known under several (code) names, then we should create duplicate entries, so that automated tools will not ignore such an entry. +### What if I don't know the module name? + +If you have a URL that is being attacked but don't know what module it belongs to, submit it but leave the name "`?`". It will be backfilled when the actual module is identified. + ### There are multiple sources, which should I use? If the vendor has issued a security statement, that should be leading. Otherwise, a statement by a security researcher (Blog/Twitter) can be used. If a vendor has issued a statement that is false or misleading, an independent statement should take precedence. @@ -89,9 +93,9 @@ If the vendor has issued a security statement, that should be leading. Otherwise Indeed, but the main advantage of a simple CSV with few columns is that it's easy to browse, maintain and extend. Other projects have stalled because there is too much overhead in vulnerability administration. The primary objective of this repository is to support a n98-magerun command. If people want more information, they can look it up via the referenced source. -### What is the URL column for? +### What is the Relevant URI column for? -This can be used by tools to filter "suspicious" web traffic from the logs. Ie, check if malicious activity has already taken place. +This can be used by tools to filter "suspicious" web traffic from the logs, for example to check if malicious activity has already taken place. The URI should be enough to uniquely match the module's vulnerable URL(s), if possible. ### What if there are multiple relevant URLs? diff --git a/src/ModuleBlacklist/Magerun/SecurityScanCommand.php b/src/ModuleBlacklist/Magerun/SecurityScanCommand.php index 457d8a9..ba7786d 100644 --- a/src/ModuleBlacklist/Magerun/SecurityScanCommand.php +++ b/src/ModuleBlacklist/Magerun/SecurityScanCommand.php @@ -127,6 +127,8 @@ protected function checkIsInstalledRoute(OutputInterface $output, \Varien_Object { $module = $this->getModuleByRoute($row->getFrontname()); + // No match if there's no module matching the frontname, or if we know what module it is for. + // Those will match by module name, if they're related. if ($module === null || ($row->getName() !== '?' && !empty($row->getName()))) { return false; } @@ -204,6 +206,26 @@ protected function getModuleVersion($moduleTag) : null; } + /** + * Get the frontname from the given (assumed) Magento route URL. + * + * @param string $route + * @return string + */ + protected function getFrontname($route) + { + // Strip off any leading index.php and slashes. A frontname shouldn't contain either. + $route = str_replace('index.php', '', $route); + $route = trim($route, '/?'); + + // If this looks like a multi-part route, the frontname is the first part. + if (strpos($route, '/') !== false) { + $route = substr($route, 0, strpos($route, '/')); + } + + return $route; + } + /** * Turn a MageVulnDb M1 CSV row into a keyed Varien_Object. * @@ -216,14 +238,12 @@ protected function getRowObject($csvRow) return false; } - $route = trim($csvRow[2], '/'); - return new \Varien_Object(array( 'name' => $csvRow[0], 'version' => $this->getModuleVersion($csvRow[0]), 'fixed_in' => $csvRow[1], - 'route' => $route, - 'frontname' => strpos($route, '/') ? substr($route, 0, strpos($route, '/')) : $route, + 'route' => $csvRow[2], + 'frontname' => $this->getFrontname($csvRow[2]), 'credit' => $csvRow[3], 'update_url' => $csvRow[4], ));