diff --git a/CHANGELOG.md b/CHANGELOG.md index 8306c5332..a35e44bcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,13 @@ * Fixed symlink creation in linux VM guest filesystems to be recognized by Windows (#10592) * Performance improvement in pool optimization step (#10585) +### [2.2.12] 2022-04-13 + + * Security: Fixed command injection vulnerability in HgDriver/GitDriver (GHSA-x7cr-6qr6-2hh6 / CVE-2022-24828) + * Fixed curl downloader not retrying when a DNS resolution failure occurs (#10716) + * Fixed composer.lock file still being used/read when the `lock` config option is disabled (#10726) + * Fixed `validate` command checking the lock file even if the `lock` option is disabled (#10723) + ### [2.2.11] 2022-04-01 * Added missing config.bitbucket-oauth in composer-schema.json @@ -1491,6 +1498,7 @@ [2.3.0]: https://github.com/composer/composer/compare/2.3.0-RC2...2.3.0 [2.3.0-RC2]: https://github.com/composer/composer/compare/2.3.0-RC1...2.3.0-RC2 [2.3.0-RC1]: https://github.com/composer/composer/compare/2.2.9...2.3.0-RC1 +[2.2.12]: https://github.com/composer/composer/compare/2.2.11...2.2.12 [2.2.11]: https://github.com/composer/composer/compare/2.2.10...2.2.11 [2.2.10]: https://github.com/composer/composer/compare/2.2.9...2.2.10 [2.2.9]: https://github.com/composer/composer/compare/2.2.8...2.2.9 diff --git a/doc/06-config.md b/doc/06-config.md index 636cf9555..5217b18f2 100644 --- a/doc/06-config.md +++ b/doc/06-config.md @@ -315,8 +315,8 @@ with other autoloaders. ## autoloader-suffix -Defaults to `null`. String to be used as a suffix for the generated Composer -autoloader. When null a random one will be generated. +Defaults to `null`. Non-empty string to be used as a suffix for the generated +Composer autoloader. When null a random one will be generated. ## optimize-autoloader @@ -396,7 +396,7 @@ in the Composer home, cache, and data directories. ## lock Defaults to `true`. If set to `false`, Composer will not create a `composer.lock` -file. +file and will ignore it if one is present. ## platform-check diff --git a/phpstan/baseline.neon b/phpstan/baseline.neon index cf5726ced..760111d2a 100644 --- a/phpstan/baseline.neon +++ b/phpstan/baseline.neon @@ -60,14 +60,9 @@ parameters: count: 1 path: ../src/Composer/Autoload/AutoloadGenerator.php - - - message: "#^Only booleans are allowed in a negated boolean, mixed given\\.$#" - count: 1 - path: ../src/Composer/Autoload/AutoloadGenerator.php - - message: "#^Only booleans are allowed in a negated boolean, string given\\.$#" - count: 4 + count: 3 path: ../src/Composer/Autoload/AutoloadGenerator.php - @@ -147,7 +142,7 @@ parameters: - message: "#^Short ternary operator is not allowed\\. Use null coalesce operator if applicable or consider using long ternary\\.$#" - count: 2 + count: 1 path: ../src/Composer/Autoload/AutoloadGenerator.php - @@ -1285,6 +1280,11 @@ parameters: count: 3 path: ../src/Composer/Command/UpdateCommand.php + - + message: "#^Only booleans are allowed in &&, mixed given on the right side\\.$#" + count: 1 + path: ../src/Composer/Command/ValidateCommand.php + - message: "#^Only booleans are allowed in a negated boolean, array\\ given\\.$#" count: 1 @@ -1370,11 +1370,6 @@ parameters: count: 1 path: ../src/Composer/Config.php - - - message: "#^Only booleans are allowed in an if condition, Composer\\\\IO\\\\IOInterface\\|null given\\.$#" - count: 1 - path: ../src/Composer/Config.php - - message: "#^Only booleans are allowed in an if condition, bool\\|string given\\.$#" count: 1 @@ -2550,11 +2545,21 @@ parameters: count: 3 path: ../src/Composer/Factory.php + - + message: "#^Only booleans are allowed in a negated boolean, mixed given\\.$#" + count: 1 + path: ../src/Composer/Factory.php + - message: "#^Only booleans are allowed in a negated boolean, string\\|false given\\.$#" count: 3 path: ../src/Composer/Factory.php + - + message: "#^Only booleans are allowed in a ternary operator condition, mixed given\\.$#" + count: 1 + path: ../src/Composer/Factory.php + - message: "#^Only booleans are allowed in an if condition, Composer\\\\Util\\\\ProcessExecutor\\|null given\\.$#" count: 1 diff --git a/src/Composer/Autoload/AutoloadGenerator.php b/src/Composer/Autoload/AutoloadGenerator.php index 4ed43d3ec..3496e1dc4 100644 --- a/src/Composer/Autoload/AutoloadGenerator.php +++ b/src/Composer/Autoload/AutoloadGenerator.php @@ -159,12 +159,11 @@ class AutoloadGenerator /** * @param string $targetDir * @param bool $scanPsrPackages - * @param string $suffix * @return int * @throws \Seld\JsonLint\ParsingException * @throws \RuntimeException */ - public function dump(Config $config, InstalledRepositoryInterface $localRepo, RootPackageInterface $rootPackage, InstallationManager $installationManager, string $targetDir, bool $scanPsrPackages = false, string $suffix = '') + public function dump(Config $config, InstalledRepositoryInterface $localRepo, RootPackageInterface $rootPackage, InstallationManager $installationManager, string $targetDir, bool $scanPsrPackages = false, ?string $suffix = null) { if ($this->classMapAuthoritative) { // Force scanPsrPackages when classmap is authoritative @@ -373,16 +372,23 @@ EOF; } $classmapFile .= ");\n"; - if (!$suffix) { - if (!$config->get('autoloader-suffix') && Filesystem::isReadable($vendorPath.'/autoload.php')) { + if ('' === $suffix) { + $suffix = null; + } + if (null === $suffix) { + $suffix = $config->get('autoloader-suffix'); + + // carry over existing autoload.php's suffix if possible and none is configured + if (null === $suffix && Filesystem::isReadable($vendorPath.'/autoload.php')) { $content = file_get_contents($vendorPath.'/autoload.php'); if (Preg::isMatch('{ComposerAutoloaderInit([^:\s]+)::}', $content, $match)) { $suffix = $match[1]; } } - if (!$suffix) { - $suffix = $config->get('autoloader-suffix') ?: md5(uniqid('', true)); + // generate one if we still haven't got a suffix + if (null === $suffix) { + $suffix = md5(uniqid('', true)); } } diff --git a/src/Composer/Command/ValidateCommand.php b/src/Composer/Command/ValidateCommand.php index b1fac8d59..80c65a1b5 100644 --- a/src/Composer/Command/ValidateCommand.php +++ b/src/Composer/Command/ValidateCommand.php @@ -45,6 +45,7 @@ class ValidateCommand extends BaseCommand ->setDescription('Validates a composer.json and composer.lock.') ->setDefinition(array( new InputOption('no-check-all', null, InputOption::VALUE_NONE, 'Do not validate requires for overly strict/loose constraints'), + new InputOption('check-lock', null, InputOption::VALUE_NONE, 'Check if lock file is up to date (even when config.lock is false)'), new InputOption('no-check-lock', null, InputOption::VALUE_NONE, 'Do not check if lock file is up to date'), new InputOption('no-check-publish', null, InputOption::VALUE_NONE, 'Do not check for publish errors'), new InputOption('no-check-version', null, InputOption::VALUE_NONE, 'Do not report a warning if the version field is present'), @@ -92,6 +93,8 @@ EOT $lockErrors = array(); $composer = Factory::create($io, $file, $input->hasParameterOption('--no-plugins')); + // config.lock = false ~= implicit --no-check-lock; --check-lock overrides + $checkLock = ($checkLock && $composer->getConfig()->get('lock')) || $input->getOption('check-lock'); $locker = $composer->getLocker(); if ($locker->isLocked() && !$locker->isFresh()) { $lockErrors[] = '- The lock file is not up to date with the latest changes in composer.json, it is recommended that you run `composer update` or `composer update `.'; diff --git a/src/Composer/Config.php b/src/Composer/Config.php index adc7e6021..182797431 100644 --- a/src/Composer/Config.php +++ b/src/Composer/Config.php @@ -427,6 +427,13 @@ class Config return $protos; + case 'autoloader-suffix': + if ($this->config[$key] === '') { // we need to guarantee null or non-empty-string + return null; + } + + return $this->process($this->config[$key], $flags); + default: if (!isset($this->config[$key])) { return null; diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index 097027348..d84d9ee88 100644 --- a/src/Composer/Factory.php +++ b/src/Composer/Factory.php @@ -418,8 +418,11 @@ class Factory // init locker if possible if ($composer instanceof Composer && isset($composerFile)) { $lockFile = self::getLockFile($composerFile); + if (!$config->get('lock') && file_exists($lockFile)) { + $io->writeError(''.$lockFile.' is present but ignored as the "lock" config option is disabled.'); + } - $locker = new Package\Locker($io, new JsonFile($lockFile, null, $io), $im, file_get_contents($composerFile), $process); + $locker = new Package\Locker($io, new JsonFile($config->get('lock') ? $lockFile : Platform::getDevNull(), null, $io), $im, file_get_contents($composerFile), $process); $composer->setLocker($locker); } diff --git a/src/Composer/Repository/Vcs/GitDriver.php b/src/Composer/Repository/Vcs/GitDriver.php index 536fd9404..6b4635c3a 100644 --- a/src/Composer/Repository/Vcs/GitDriver.php +++ b/src/Composer/Repository/Vcs/GitDriver.php @@ -145,6 +145,10 @@ class GitDriver extends VcsDriver */ public function getFileContent(string $file, string $identifier): ?string { + if (isset($identifier[0]) && $identifier[0] === '-') { + throw new \RuntimeException('Invalid git identifier detected. Identifier must not start with a -, given: ' . $identifier); + } + $resource = sprintf('%s:%s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file)); $this->process->execute(sprintf('git show %s', $resource), $content, $this->repoDir); @@ -198,7 +202,7 @@ class GitDriver extends VcsDriver $this->process->execute('git branch --no-color --no-abbrev -v', $output, $this->repoDir); foreach ($this->process->splitLines($output) as $branch) { if ($branch && !Preg::isMatch('{^ *[^/]+/HEAD }', $branch)) { - if (Preg::isMatch('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match)) { + if (Preg::isMatch('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match) && $match[1][0] !== '-') { $branches[$match[1]] = $match[2]; } } diff --git a/src/Composer/Repository/Vcs/HgDriver.php b/src/Composer/Repository/Vcs/HgDriver.php index 4cc9e8d1e..33515f00e 100644 --- a/src/Composer/Repository/Vcs/HgDriver.php +++ b/src/Composer/Repository/Vcs/HgDriver.php @@ -126,7 +126,11 @@ class HgDriver extends VcsDriver */ public function getFileContent(string $file, string $identifier): ?string { - $resource = sprintf('hg cat -r %s %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file)); + if (isset($identifier[0]) && $identifier[0] === '-') { + throw new \RuntimeException('Invalid hg identifier detected. Identifier must not start with a -, given: ' . $identifier); + } + + $resource = sprintf('hg cat -r %s -- %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file)); $this->process->execute($resource, $content, $this->repoDir); if (!trim($content)) { @@ -186,14 +190,14 @@ class HgDriver extends VcsDriver $this->process->execute('hg branches', $output, $this->repoDir); foreach ($this->process->splitLines($output) as $branch) { - if ($branch && Preg::isMatch('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match)) { + if ($branch && Preg::isMatch('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match) && $match[1][0] !== '-') { $branches[$match[1]] = $match[2]; } } $this->process->execute('hg bookmarks', $output, $this->repoDir); foreach ($this->process->splitLines($output) as $branch) { - if ($branch && Preg::isMatch('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match)) { + if ($branch && Preg::isMatch('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match) && $match[1][0] !== '-') { $bookmarks[$match[1]] = $match[2]; } } diff --git a/src/Composer/Util/Platform.php b/src/Composer/Util/Platform.php index eb71673a2..be37ba621 100644 --- a/src/Composer/Util/Platform.php +++ b/src/Composer/Util/Platform.php @@ -275,4 +275,16 @@ class Platform return self::$isVirtualBoxGuest; } + + /** + * @return 'NUL'|'/dev/null' + */ + public static function getDevNull(): string + { + if (self::isWindows()) { + return 'NUL'; + } + + return '/dev/null'; + } } diff --git a/tests/Composer/Test/Repository/Vcs/GitDriverTest.php b/tests/Composer/Test/Repository/Vcs/GitDriverTest.php index 87080a4b2..1fb52f3be 100644 --- a/tests/Composer/Test/Repository/Vcs/GitDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/GitDriverTest.php @@ -8,6 +8,7 @@ use Composer\Repository\Vcs\GitDriver; use Composer\Test\TestCase; use Composer\Util\Filesystem; use Composer\Util\Platform; +use Composer\Test\Mock\ProcessExecutorMock; class GitDriverTest extends TestCase { @@ -132,6 +133,48 @@ GIT; $this->assertSame('main', $driver->getRootIdentifier()); } + public function testGetBranchesFilterInvalidBranchNames(): void + { + $process = $this->getProcessExecutorMock(); + $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(); + + $driver = new GitDriver(array('url' => 'https://example.org/acme.git'), $io, $this->config, $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock(), $process); + $this->setRepoDir($driver, $this->home); + + // Branches starting with a - character are not valid git branches names + // Still assert that they get filtered to prevent issues later on + $stdout = <<expects(array(array( + 'cmd' => 'git branch --no-color --no-abbrev -v', + 'stdout' => $stdout, + ))); + + $branches = $driver->getBranches(); + $this->assertSame(array( + 'main' => '089681446ba44d6d9004350192486f2ceb4eaa06', + '2.2' => '12681446ba44d6d9004350192486f2ceb4eaa06', + ), $branches); + } + + public function testFileGetContentInvalidIdentifier(): void + { + $this->expectException('\RuntimeException'); + + $process = $this->getProcessExecutorMock(); + $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(); + $driver = new GitDriver(array('url' => 'https://example.org/acme.git'), $io, $this->config, $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock(), $process); + + $this->assertNull($driver->getFileContent('file.txt', 'h')); + + $driver->getFileContent('file.txt', '-h'); + } + private function setRepoDir(GitDriver $driver, string $path): void { $reflectionClass = new \ReflectionClass($driver); diff --git a/tests/Composer/Test/Repository/Vcs/HgDriverTest.php b/tests/Composer/Test/Repository/Vcs/HgDriverTest.php index 58cac0c16..a65ad1526 100644 --- a/tests/Composer/Test/Repository/Vcs/HgDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/HgDriverTest.php @@ -13,6 +13,7 @@ namespace Composer\Test\Repository\Vcs; use Composer\Repository\Vcs\HgDriver; +use Composer\Test\Mock\ProcessExecutorMock; use Composer\Test\TestCase; use Composer\Util\Filesystem; use Composer\Config; @@ -67,4 +68,49 @@ class HgDriverTest extends TestCase array('https://user@bitbucket.org/user/repo'), ); } + + public function testGetBranchesFilterInvalidBranchNames(): void + { + $process = $this->getProcessExecutorMock(); + + $driver = new HgDriver(array('url' => 'https://example.org/acme.git'), $this->io, $this->config, $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock(), $process); + + $stdout = <<expects(array(array( + 'cmd' => 'hg branches', + 'stdout' => $stdout, + ), array( + 'cmd' => 'hg bookmarks', + 'stdout' => $stdout1, + ))); + + $branches = $driver->getBranches(); + $this->assertSame(array( + 'help' => 'dbf6c8acb641', + 'default' => 'dbf6c8acb640', + ), $branches); + } + + public function testFileGetContentInvalidIdentifier(): void + { + $this->expectException('\RuntimeException'); + + $process = $this->getProcessExecutorMock(); + $driver = new HgDriver(array('url' => 'https://example.org/acme.git'), $this->io, $this->config, $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock(), $process); + + $this->assertNull($driver->getFileContent('file.txt', 'h')); + + $driver->getFileContent('file.txt', '-h'); + } }