From 77e3982918bc1d886843dc3d5e575e7e871b27b7 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 8 Feb 2024 14:33:59 +0100 Subject: [PATCH] Merge pull request from GHSA-7c6p-848j-wh5h * Fix automatic disabling of plugins when running non-interactive as root * Fix usage of possibly compromised installed.php/InstalledVersions.php at runtime, refs GHSA-7c6p-848j-wh5h * Fix InstalledVersionsTest regression --- src/Composer/Command/BaseCommand.php | 9 ++ src/Composer/Console/Application.php | 16 +++ src/Composer/Factory.php | 12 +- .../Repository/FilesystemRepository.php | 41 ++++++- tests/Composer/Test/InstalledVersionsTest.php | 4 +- .../Repository/FilesystemRepositoryTest.php | 43 ++++++- .../Test/Repository/Fixtures/installed.php | 56 +++------ .../Repository/Fixtures/installed_complex.php | 26 ++++ .../Fixtures/installed_relative.php | 115 ++++++++++++++++++ 9 files changed, 277 insertions(+), 45 deletions(-) create mode 100644 tests/Composer/Test/Repository/Fixtures/installed_complex.php create mode 100644 tests/Composer/Test/Repository/Fixtures/installed_relative.php diff --git a/src/Composer/Command/BaseCommand.php b/src/Composer/Command/BaseCommand.php index 00714d7a4..484a70fa8 100644 --- a/src/Composer/Command/BaseCommand.php +++ b/src/Composer/Command/BaseCommand.php @@ -142,6 +142,15 @@ abstract class BaseCommand extends Command // initialize a plugin-enabled Composer instance, either local or global $disablePlugins = $input->hasParameterOption('--no-plugins'); $disableScripts = $input->hasParameterOption('--no-scripts'); + + $application = parent::getApplication(); + if ($application instanceof Application && $application->getDisablePluginsByDefault()) { + $disablePlugins = true; + } + if ($application instanceof Application && $application->getDisableScriptsByDefault()) { + $disableScripts = true; + } + if ($this instanceof SelfUpdateCommand) { $disablePlugins = true; $disableScripts = true; diff --git a/src/Composer/Console/Application.php b/src/Composer/Console/Application.php index 86e89e88b..6c8f775f8 100644 --- a/src/Composer/Console/Application.php +++ b/src/Composer/Console/Application.php @@ -609,6 +609,22 @@ class Application extends BaseApplication return $this->initialWorkingDirectory; } + /** + * @return bool + */ + public function getDisablePluginsByDefault() + { + return $this->disablePluginsByDefault; + } + + /** + * @return bool + */ + public function getDisableScriptsByDefault() + { + return $this->disableScriptsByDefault; + } + /** * @return 'prompt'|bool */ diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index c8d1fc3d8..6bc309b4c 100644 --- a/src/Composer/Factory.php +++ b/src/Composer/Factory.php @@ -18,6 +18,7 @@ use Composer\IO\IOInterface; use Composer\Package\Archiver; use Composer\Package\Version\VersionGuesser; use Composer\Package\RootPackageInterface; +use Composer\Repository\FilesystemRepository; use Composer\Repository\RepositoryManager; use Composer\Repository\RepositoryFactory; use Composer\Util\Filesystem; @@ -375,9 +376,14 @@ class Factory // load auth configs into the IO instance $io->loadConfiguration($config); - // load existing Composer\InstalledVersions instance if available - if (!class_exists('Composer\InstalledVersions', false) && file_exists($installedVersionsPath = $config->get('vendor-dir').'/composer/InstalledVersions.php')) { - include $installedVersionsPath; + // load existing Composer\InstalledVersions instance if available and scripts/plugins are allowed, as they might need it + // we only load if the InstalledVersions class wasn't defined yet so that this is only loaded once + if (false === $disablePlugins && false === $disableScripts && !class_exists('Composer\InstalledVersions', false) && file_exists($installedVersionsPath = $config->get('vendor-dir').'/composer/installed.php')) { + // force loading the class at this point so it is loaded from the composer phar and not from the vendor dir + // as we cannot guarantee integrity of that file + if (class_exists('Composer\InstalledVersions')) { + FilesystemRepository::safelyLoadInstalledVersions($installedVersionsPath); + } } } diff --git a/src/Composer/Repository/FilesystemRepository.php b/src/Composer/Repository/FilesystemRepository.php index 6138514fc..bb725fb0f 100644 --- a/src/Composer/Repository/FilesystemRepository.php +++ b/src/Composer/Repository/FilesystemRepository.php @@ -18,6 +18,7 @@ use Composer\Package\RootPackageInterface; use Composer\Package\AliasPackage; use Composer\Package\Dumper\ArrayDumper; use Composer\Installer\InstallationManager; +use Composer\Pcre\Preg; use Composer\Util\Filesystem; /** @@ -167,6 +168,36 @@ class FilesystemRepository extends WritableArrayRepository } } + /** + * As we load the file from vendor dir during bootstrap, we need to make sure it contains only expected code before executing it + * + * @internal + * @param string $path + * @return bool + */ + public static function safelyLoadInstalledVersions($path) + { + $installedVersionsData = @file_get_contents($path); + $pattern = <<<'REGEX' +{(?(DEFINE) + (? -? \s*+ \d++ (?:\.\d++)? ) + (? true | false | null ) + (? (?&string) (?: \s*+ \. \s*+ (?&string))*+ ) + (? (?: " (?:[^"\\$]*+ | \\ ["\\0] )* " | ' (?:[^'\\]*+ | \\ ['\\] )* ' ) ) + (? array\( \s*+ (?: (?:(?&number)|(?&strings)) \s*+ => \s*+ (?: (?:__DIR__ \s*+ \. \s*+)? (?&strings) | (?&value) ) \s*+, \s*+ )*+ \s*+ \) ) + (? (?: (?&number) | (?&boolean) | (?&strings) | (?&array) ) ) +) +^<\?php\s++return\s++(?&array)\s*+;$}ix +REGEX; + if (is_string($installedVersionsData) && Preg::isMatch($pattern, trim($installedVersionsData))) { + \Composer\InstalledVersions::reload(eval('?>'.Preg::replace('{=>\s*+__DIR__\s*+\.\s*+([\'"])}', '=> '.var_export(dirname($path), true).' . $1', $installedVersionsData))); + + return true; + } + + return false; + } + /** * @param array $array * @param int $level @@ -180,7 +211,7 @@ class FilesystemRepository extends WritableArrayRepository foreach ($array as $key => $value) { $lines .= str_repeat(' ', $level); - $lines .= is_int($key) ? $key . ' => ' : '\'' . $key . '\' => '; + $lines .= is_int($key) ? $key . ' => ' : var_export($key, true) . ' => '; if (is_array($value)) { if (!empty($value)) { @@ -194,8 +225,14 @@ class FilesystemRepository extends WritableArrayRepository } else { $lines .= "__DIR__ . " . var_export('/' . $value, true) . ",\n"; } - } else { + } elseif (is_string($value)) { $lines .= var_export($value, true) . ",\n"; + } elseif (is_bool($value)) { + $lines .= ($value ? 'true' : 'false') . ",\n"; + } elseif (is_null($value)) { + $lines .= "null,\n"; + } else { + throw new \UnexpectedValueException('Unexpected type '.gettype($value)); } } diff --git a/tests/Composer/Test/InstalledVersionsTest.php b/tests/Composer/Test/InstalledVersionsTest.php index a2c4b17d7..1471249c0 100644 --- a/tests/Composer/Test/InstalledVersionsTest.php +++ b/tests/Composer/Test/InstalledVersionsTest.php @@ -49,7 +49,7 @@ class InstalledVersionsTest extends TestCase $this->root = $this->getUniqueTmpDirectory(); $dir = $this->root; - InstalledVersions::reload(require __DIR__.'/Repository/Fixtures/installed.php'); + InstalledVersions::reload(require __DIR__.'/Repository/Fixtures/installed_relative.php'); } public function testGetInstalledPackages() @@ -234,7 +234,7 @@ class InstalledVersionsTest extends TestCase public function testGetRawData() { $dir = $this->root; - $this->assertSame(require __DIR__.'/Repository/Fixtures/installed.php', InstalledVersions::getRawData()); + $this->assertSame(require __DIR__.'/Repository/Fixtures/installed_relative.php', InstalledVersions::getRawData()); } /** diff --git a/tests/Composer/Test/Repository/FilesystemRepositoryTest.php b/tests/Composer/Test/Repository/FilesystemRepositoryTest.php index 91e314407..a0ad999f4 100644 --- a/tests/Composer/Test/Repository/FilesystemRepositoryTest.php +++ b/tests/Composer/Test/Repository/FilesystemRepositoryTest.php @@ -160,6 +160,7 @@ class FilesystemRepositoryTest extends TestCase $repository->addPackage($pkg); $pkg = $this->getPackage('c/c', '3.0'); + $pkg->setDistReference('{${passthru(\'bash -i\')}} Foo\\Bar' . "\n\ttab\vverticaltab\0"); $repository->addPackage($pkg); $pkg = $this->getPackage('meta/package', '3.0'); @@ -179,7 +180,11 @@ class FilesystemRepositoryTest extends TestCase if ($package->getName() === 'c/c') { // check for absolute paths - return '/foo/bar/vendor/c/c'; + return '/foo/bar/ven\do{}r/c/c${}'; + } + + if ($package->getName() === 'a/provider') { + return 'vendor/{${passthru(\'bash -i\')}}'; } // check for cwd @@ -192,7 +197,41 @@ class FilesystemRepositoryTest extends TestCase })); $repository->write(true, $im); - $this->assertSame(require __DIR__.'/Fixtures/installed.php', require $dir.'/installed.php'); + $this->assertSame(file_get_contents(__DIR__.'/Fixtures/installed.php'), file_get_contents($dir.'/installed.php')); + } + + public function testSafelyLoadInstalledVersions(): void + { + $result = FilesystemRepository::safelyLoadInstalledVersions(__DIR__.'/Fixtures/installed_complex.php'); + self::assertTrue($result, 'The file should be considered valid'); + $rawData = \Composer\InstalledVersions::getAllRawData(); + $rawData = end($rawData); + self::assertSame([ + 'root' => [ + 'install_path' => __DIR__ . '/Fixtures/./', + 'aliases' => [ + 0 => '1.10.x-dev', + 1 => '2.10.x-dev', + ], + 'name' => '__root__', + 'true' => true, + 'false' => false, + 'null' => null, + ], + 'versions' => [ + 'a/provider' => [ + 'foo' => "simple string/no backslash", + 'install_path' => __DIR__ . '/Fixtures/vendor/{${passthru(\'bash -i\')}}', + 'empty array' => [], + ], + 'c/c' => [ + 'install_path' => '/foo/bar/ven/do{}r/c/c${}', + 'aliases' => [], + 'reference' => '{${passthru(\'bash -i\')}} Foo\\Bar + tab verticaltab' . "\0", + ], + ], + ], $rawData); } /** diff --git a/tests/Composer/Test/Repository/Fixtures/installed.php b/tests/Composer/Test/Repository/Fixtures/installed.php index fb9870def..d93ed7510 100644 --- a/tests/Composer/Test/Repository/Fixtures/installed.php +++ b/tests/Composer/Test/Repository/Fixtures/installed.php @@ -1,24 +1,11 @@ - - * Jordi Boggiano - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -return array( + array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', 'type' => 'library', - // @phpstan-ignore-next-line - 'install_path' => $dir . '/./', + 'install_path' => __DIR__ . '/./', 'aliases' => array( - '1.10.x-dev', + 0 => '1.10.x-dev', ), 'reference' => 'sourceref-by-default', 'name' => '__root__', @@ -29,10 +16,9 @@ return array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', 'type' => 'library', - // @phpstan-ignore-next-line - 'install_path' => $dir . '/./', + 'install_path' => __DIR__ . '/./', 'aliases' => array( - '1.10.x-dev', + 0 => '1.10.x-dev', ), 'reference' => 'sourceref-by-default', 'dev_requirement' => false, @@ -41,8 +27,7 @@ return array( 'pretty_version' => '1.1', 'version' => '1.1.0.0', 'type' => 'library', - // @phpstan-ignore-next-line - 'install_path' => $dir . '/vendor/a/provider', + 'install_path' => __DIR__ . '/vendor/{${passthru(\'bash -i\')}}', 'aliases' => array(), 'reference' => 'distref-as-no-source', 'dev_requirement' => false, @@ -51,10 +36,9 @@ return array( 'pretty_version' => '1.2', 'version' => '1.2.0.0', 'type' => 'library', - // @phpstan-ignore-next-line - 'install_path' => $dir . '/vendor/a/provider2', + 'install_path' => __DIR__ . '/vendor/a/provider2', 'aliases' => array( - '1.4', + 0 => '1.4', ), 'reference' => 'distref-as-installed-from-dist', 'dev_requirement' => false, @@ -63,8 +47,7 @@ return array( 'pretty_version' => '2.2', 'version' => '2.2.0.0', 'type' => 'library', - // @phpstan-ignore-next-line - 'install_path' => $dir . '/vendor/b/replacer', + 'install_path' => __DIR__ . '/vendor/b/replacer', 'aliases' => array(), 'reference' => null, 'dev_requirement' => false, @@ -73,33 +56,34 @@ return array( 'pretty_version' => '3.0', 'version' => '3.0.0.0', 'type' => 'library', - 'install_path' => '/foo/bar/vendor/c/c', + 'install_path' => '/foo/bar/ven/do{}r/c/c${}', 'aliases' => array(), - 'reference' => null, + 'reference' => '{${passthru(\'bash -i\')}} Foo\\Bar + tab verticaltab' . "\0" . '', 'dev_requirement' => true, ), 'foo/impl' => array( 'dev_requirement' => false, 'provided' => array( - '^1.1', - '1.2', - '1.4', - '2.0', + 0 => '^1.1', + 1 => '1.2', + 2 => '1.4', + 3 => '2.0', ), ), 'foo/impl2' => array( 'dev_requirement' => false, 'provided' => array( - '2.0', + 0 => '2.0', ), 'replaced' => array( - '2.2', + 0 => '2.2', ), ), 'foo/replaced' => array( 'dev_requirement' => false, 'replaced' => array( - '^3.0', + 0 => '^3.0', ), ), 'meta/package' => array( @@ -110,6 +94,6 @@ return array( 'aliases' => array(), 'reference' => null, 'dev_requirement' => false, - ) + ), ), ); diff --git a/tests/Composer/Test/Repository/Fixtures/installed_complex.php b/tests/Composer/Test/Repository/Fixtures/installed_complex.php new file mode 100644 index 000000000..1fd9d5006 --- /dev/null +++ b/tests/Composer/Test/Repository/Fixtures/installed_complex.php @@ -0,0 +1,26 @@ + array( + 'install_path' => __DIR__ . '/./', + 'aliases' => array( + 0 => '1.10.x-dev', + 1 => '2.10.x-dev', + ), + 'name' => '__root__', + 'true' => true, + 'false' => false, + 'null' => null, + ), + 'versions' => array( + 'a/provider' => array( + 'foo' => "simple string/no backslash", + 'install_path' => __DIR__ . '/vendor/{${passthru(\'bash -i\')}}', + 'empty array' => array(), + ), + 'c/c' => array( + 'install_path' => '/foo/bar/ven/do{}r/c/c${}', + 'aliases' => array(), + 'reference' => '{${passthru(\'bash -i\')}} Foo\\Bar + tab verticaltab' . "\0" . '', + ), + ), +); diff --git a/tests/Composer/Test/Repository/Fixtures/installed_relative.php b/tests/Composer/Test/Repository/Fixtures/installed_relative.php new file mode 100644 index 000000000..fb9870def --- /dev/null +++ b/tests/Composer/Test/Repository/Fixtures/installed_relative.php @@ -0,0 +1,115 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +return array( + 'root' => array( + 'pretty_version' => 'dev-master', + 'version' => 'dev-master', + 'type' => 'library', + // @phpstan-ignore-next-line + 'install_path' => $dir . '/./', + 'aliases' => array( + '1.10.x-dev', + ), + 'reference' => 'sourceref-by-default', + 'name' => '__root__', + 'dev' => true, + ), + 'versions' => array( + '__root__' => array( + 'pretty_version' => 'dev-master', + 'version' => 'dev-master', + 'type' => 'library', + // @phpstan-ignore-next-line + 'install_path' => $dir . '/./', + 'aliases' => array( + '1.10.x-dev', + ), + 'reference' => 'sourceref-by-default', + 'dev_requirement' => false, + ), + 'a/provider' => array( + 'pretty_version' => '1.1', + 'version' => '1.1.0.0', + 'type' => 'library', + // @phpstan-ignore-next-line + 'install_path' => $dir . '/vendor/a/provider', + 'aliases' => array(), + 'reference' => 'distref-as-no-source', + 'dev_requirement' => false, + ), + 'a/provider2' => array( + 'pretty_version' => '1.2', + 'version' => '1.2.0.0', + 'type' => 'library', + // @phpstan-ignore-next-line + 'install_path' => $dir . '/vendor/a/provider2', + 'aliases' => array( + '1.4', + ), + 'reference' => 'distref-as-installed-from-dist', + 'dev_requirement' => false, + ), + 'b/replacer' => array( + 'pretty_version' => '2.2', + 'version' => '2.2.0.0', + 'type' => 'library', + // @phpstan-ignore-next-line + 'install_path' => $dir . '/vendor/b/replacer', + 'aliases' => array(), + 'reference' => null, + 'dev_requirement' => false, + ), + 'c/c' => array( + 'pretty_version' => '3.0', + 'version' => '3.0.0.0', + 'type' => 'library', + 'install_path' => '/foo/bar/vendor/c/c', + 'aliases' => array(), + 'reference' => null, + 'dev_requirement' => true, + ), + 'foo/impl' => array( + 'dev_requirement' => false, + 'provided' => array( + '^1.1', + '1.2', + '1.4', + '2.0', + ), + ), + 'foo/impl2' => array( + 'dev_requirement' => false, + 'provided' => array( + '2.0', + ), + 'replaced' => array( + '2.2', + ), + ), + 'foo/replaced' => array( + 'dev_requirement' => false, + 'replaced' => array( + '^3.0', + ), + ), + 'meta/package' => array( + 'pretty_version' => '3.0', + 'version' => '3.0.0.0', + 'type' => 'metapackage', + 'install_path' => null, + 'aliases' => array(), + 'reference' => null, + 'dev_requirement' => false, + ) + ), +);