From 083b73515d1d72bc61c6374440b3f8a37531f8cf Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 27 Apr 2021 13:00:40 +0200 Subject: [PATCH 1/2] Merge pull request from GHSA-h5h8-pc6h-jvvx * Fix external process calls to avoid user input being able to pass extra parameters * Tweak some fixes --- src/Composer/Downloader/FossilDownloader.php | 6 ++-- src/Composer/Downloader/GitDownloader.php | 10 +++--- src/Composer/Downloader/GzipDownloader.php | 2 +- src/Composer/Downloader/HgDownloader.php | 6 ++-- src/Composer/Downloader/RarDownloader.php | 2 +- src/Composer/Downloader/SvnDownloader.php | 2 +- src/Composer/Repository/Vcs/FossilDriver.php | 6 ++-- src/Composer/Repository/Vcs/GitDriver.php | 2 +- src/Composer/Repository/Vcs/HgDriver.php | 4 +-- src/Composer/Repository/Vcs/SvnDriver.php | 2 +- src/Composer/Util/Git.php | 4 +-- src/Composer/Util/Svn.php | 2 +- .../Test/Downloader/FossilDownloaderTest.php | 6 ++-- .../Test/Downloader/GitDownloaderTest.php | 34 +++++++++---------- .../Test/Downloader/HgDownloaderTest.php | 6 ++-- tests/Composer/Test/Util/SvnTest.php | 2 +- 16 files changed, 48 insertions(+), 48 deletions(-) diff --git a/src/Composer/Downloader/FossilDownloader.php b/src/Composer/Downloader/FossilDownloader.php index 135e973e0..57ee85965 100644 --- a/src/Composer/Downloader/FossilDownloader.php +++ b/src/Composer/Downloader/FossilDownloader.php @@ -32,15 +32,15 @@ class FossilDownloader extends VcsDownloader $ref = ProcessExecutor::escape($package->getSourceReference()); $repoFile = $path . '.fossil'; $this->io->writeError("Cloning ".$package->getSourceReference()); - $command = sprintf('fossil clone %s %s', $url, ProcessExecutor::escape($repoFile)); + $command = sprintf('fossil clone -- %s %s', $url, ProcessExecutor::escape($repoFile)); if (0 !== $this->process->execute($command, $ignoredOutput)) { throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); } - $command = sprintf('fossil open %s --nested', ProcessExecutor::escape($repoFile)); + $command = sprintf('fossil open --nested -- %s', ProcessExecutor::escape($repoFile)); if (0 !== $this->process->execute($command, $ignoredOutput, realpath($path))) { throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); } - $command = sprintf('fossil update %s', $ref); + $command = sprintf('fossil update -- %s', $ref); if (0 !== $this->process->execute($command, $ignoredOutput, realpath($path))) { throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); } diff --git a/src/Composer/Downloader/GitDownloader.php b/src/Composer/Downloader/GitDownloader.php index ad8503cea..88cb4b475 100644 --- a/src/Composer/Downloader/GitDownloader.php +++ b/src/Composer/Downloader/GitDownloader.php @@ -51,7 +51,7 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface $gitVersion = GitUtil::getVersion($this->process); $msg = "Cloning ".$this->getShortHash($ref); - $command = 'git clone --no-checkout %url% %path% && cd '.$flag.'%path% && git remote add composer %url% && git fetch composer && git remote set-url origin %sanitizedUrl% && git remote set-url composer %sanitizedUrl%'; + $command = 'git clone --no-checkout -- %url% %path% && cd '.$flag.'%path% && git remote add composer -- %url% && git fetch composer && git remote set-url origin -- %sanitizedUrl% && git remote set-url composer -- %sanitizedUrl%'; if ($gitVersion && version_compare($gitVersion, '2.3.0-rc0', '>=') && Cache::isUsable($cachePath)) { $this->io->writeError('', true, IOInterface::DEBUG); $this->io->writeError(sprintf(' Cloning to cache at %s', ProcessExecutor::escape($cachePath)), true, IOInterface::DEBUG); @@ -63,7 +63,7 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface $command = 'git clone --no-checkout %cachePath% %path% --dissociate --reference %cachePath% ' . '&& cd '.$flag.'%path% ' - . '&& git remote set-url origin %sanitizedUrl% && git remote add composer %sanitizedUrl%'; + . '&& git remote set-url origin -- %sanitizedUrl% && git remote add composer -- %sanitizedUrl%'; $msg = "Cloning ".$this->getShortHash($ref).' from cache'; } } catch (\RuntimeException $e) { @@ -125,7 +125,7 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface $ref = $target->getSourceReference(); $this->io->writeError(" Checking out ".$this->getShortHash($ref)); - $command = '(git remote set-url composer %s && git rev-parse --quiet --verify %s || (git fetch composer && git fetch --tags composer)) && git remote set-url composer %s'; + $command = '(git remote set-url composer -- %s && git rev-parse --quiet --verify %s || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- %s'; $commandCallable = function ($url) use ($command, $ref) { return sprintf( @@ -411,7 +411,7 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface protected function updateOriginUrl($path, $url) { - $this->process->execute(sprintf('git remote set-url origin %s', ProcessExecutor::escape($url)), $output, $path); + $this->process->execute(sprintf('git remote set-url origin -- %s', ProcessExecutor::escape($url)), $output, $path); $this->setPushUrl($path, $url); } @@ -424,7 +424,7 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface if (!in_array('ssh', $protocols, true)) { $pushUrl = 'https://' . $match[1] . '/'.$match[2].'/'.$match[3].'.git'; } - $cmd = sprintf('git remote set-url --push origin %s', ProcessExecutor::escape($pushUrl)); + $cmd = sprintf('git remote set-url --push origin -- %s', ProcessExecutor::escape($pushUrl)); $this->process->execute($cmd, $ignoredOutput, $path); } } diff --git a/src/Composer/Downloader/GzipDownloader.php b/src/Composer/Downloader/GzipDownloader.php index 19e4a45e1..b7c6de3f7 100644 --- a/src/Composer/Downloader/GzipDownloader.php +++ b/src/Composer/Downloader/GzipDownloader.php @@ -42,7 +42,7 @@ class GzipDownloader extends ArchiveDownloader // Try to use gunzip on *nix if (!Platform::isWindows()) { - $command = 'gzip -cd ' . ProcessExecutor::escape($file) . ' > ' . ProcessExecutor::escape($targetFilepath); + $command = 'gzip -cd -- ' . ProcessExecutor::escape($file) . ' > ' . ProcessExecutor::escape($targetFilepath); if (0 === $this->process->execute($command, $ignoredOutput)) { return; diff --git a/src/Composer/Downloader/HgDownloader.php b/src/Composer/Downloader/HgDownloader.php index 2921cc4b7..0be3374f6 100644 --- a/src/Composer/Downloader/HgDownloader.php +++ b/src/Composer/Downloader/HgDownloader.php @@ -29,13 +29,13 @@ class HgDownloader extends VcsDownloader $hgUtils = new HgUtils($this->io, $this->config, $this->process); $cloneCommand = function ($url) use ($path) { - return sprintf('hg clone %s %s', ProcessExecutor::escape($url), ProcessExecutor::escape($path)); + return sprintf('hg clone -- %s %s', ProcessExecutor::escape($url), ProcessExecutor::escape($path)); }; $hgUtils->runCommand($cloneCommand, $url, $path); $ref = ProcessExecutor::escape($package->getSourceReference()); - $command = sprintf('hg up %s', $ref); + $command = sprintf('hg up -- %s', $ref); if (0 !== $this->process->execute($command, $ignoredOutput, realpath($path))) { throw new \RuntimeException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput()); } @@ -56,7 +56,7 @@ class HgDownloader extends VcsDownloader } $command = function ($url) use ($ref) { - return sprintf('hg pull %s && hg up %s', ProcessExecutor::escape($url), ProcessExecutor::escape($ref)); + return sprintf('hg pull -- %s && hg up -- %s', ProcessExecutor::escape($url), ProcessExecutor::escape($ref)); }; $hgUtils->runCommand($command, $url, $path); diff --git a/src/Composer/Downloader/RarDownloader.php b/src/Composer/Downloader/RarDownloader.php index 40cd09896..1a4a39619 100644 --- a/src/Composer/Downloader/RarDownloader.php +++ b/src/Composer/Downloader/RarDownloader.php @@ -45,7 +45,7 @@ class RarDownloader extends ArchiveDownloader // Try to use unrar on *nix if (!Platform::isWindows()) { - $command = 'unrar x ' . ProcessExecutor::escape($file) . ' ' . ProcessExecutor::escape($path) . ' >/dev/null && chmod -R u+w ' . ProcessExecutor::escape($path); + $command = 'unrar x -- ' . ProcessExecutor::escape($file) . ' ' . ProcessExecutor::escape($path) . ' >/dev/null && chmod -R u+w ' . ProcessExecutor::escape($path); if (0 === $this->process->execute($command, $ignoredOutput)) { return; diff --git a/src/Composer/Downloader/SvnDownloader.php b/src/Composer/Downloader/SvnDownloader.php index f7e14e6ce..4c26a6050 100644 --- a/src/Composer/Downloader/SvnDownloader.php +++ b/src/Composer/Downloader/SvnDownloader.php @@ -172,7 +172,7 @@ class SvnDownloader extends VcsDownloader { if (preg_match('{.*@(\d+)$}', $fromReference) && preg_match('{.*@(\d+)$}', $toReference)) { // retrieve the svn base url from the checkout folder - $command = sprintf('svn info --non-interactive --xml %s', ProcessExecutor::escape($path)); + $command = sprintf('svn info --non-interactive --xml -- %s', ProcessExecutor::escape($path)); if (0 !== $this->process->execute($command, $output, $path)) { throw new \RuntimeException( 'Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput() diff --git a/src/Composer/Repository/Vcs/FossilDriver.php b/src/Composer/Repository/Vcs/FossilDriver.php index 491fafa86..8839deb5e 100644 --- a/src/Composer/Repository/Vcs/FossilDriver.php +++ b/src/Composer/Repository/Vcs/FossilDriver.php @@ -95,13 +95,13 @@ class FossilDriver extends VcsDriver $fs->ensureDirectoryExists($this->checkoutDir); - if (0 !== $this->process->execute(sprintf('fossil clone %s %s', ProcessExecutor::escape($this->url), ProcessExecutor::escape($this->repoFile)), $output)) { + if (0 !== $this->process->execute(sprintf('fossil clone -- %s %s', ProcessExecutor::escape($this->url), ProcessExecutor::escape($this->repoFile)), $output)) { $output = $this->process->getErrorOutput(); throw new \RuntimeException('Failed to clone '.$this->url.' to repository ' . $this->repoFile . "\n\n" .$output); } - if (0 !== $this->process->execute(sprintf('fossil open %s --nested', ProcessExecutor::escape($this->repoFile)), $output, $this->checkoutDir)) { + if (0 !== $this->process->execute(sprintf('fossil open --nested -- %s', ProcessExecutor::escape($this->repoFile)), $output, $this->checkoutDir)) { $output = $this->process->getErrorOutput(); throw new \RuntimeException('Failed to open repository '.$this->repoFile.' in ' . $this->checkoutDir . "\n\n" .$output); @@ -150,7 +150,7 @@ class FossilDriver extends VcsDriver */ public function getFileContent($file, $identifier) { - $command = sprintf('fossil cat -r %s %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file)); + $command = sprintf('fossil cat -r %s -- %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file)); $this->process->execute($command, $content, $this->checkoutDir); if (!trim($content)) { diff --git a/src/Composer/Repository/Vcs/GitDriver.php b/src/Composer/Repository/Vcs/GitDriver.php index 2c7782c98..9e3fa23f0 100644 --- a/src/Composer/Repository/Vcs/GitDriver.php +++ b/src/Composer/Repository/Vcs/GitDriver.php @@ -227,7 +227,7 @@ class GitDriver extends VcsDriver try { $gitUtil->runCommand(function ($url) { - return 'git ls-remote --heads ' . ProcessExecutor::escape($url); + return 'git ls-remote --heads -- ' . ProcessExecutor::escape($url); }, $url, sys_get_temp_dir()); } catch (\RuntimeException $e) { return false; diff --git a/src/Composer/Repository/Vcs/HgDriver.php b/src/Composer/Repository/Vcs/HgDriver.php index 04a363442..e739d4cda 100644 --- a/src/Composer/Repository/Vcs/HgDriver.php +++ b/src/Composer/Repository/Vcs/HgDriver.php @@ -68,7 +68,7 @@ class HgDriver extends VcsDriver $repoDir = $this->repoDir; $command = function ($url) use ($repoDir) { - return sprintf('hg clone --noupdate %s %s', ProcessExecutor::escape($url), ProcessExecutor::escape($repoDir)); + return sprintf('hg clone --noupdate -- %s %s', ProcessExecutor::escape($url), ProcessExecutor::escape($repoDir)); }; $hgUtils->runCommand($command, $this->url, null); @@ -229,7 +229,7 @@ class HgDriver extends VcsDriver } $processExecutor = new ProcessExecutor($io); - $exit = $processExecutor->execute(sprintf('hg identify %s', ProcessExecutor::escape($url)), $ignored); + $exit = $processExecutor->execute(sprintf('hg identify -- %s', ProcessExecutor::escape($url)), $ignored); return $exit === 0; } diff --git a/src/Composer/Repository/Vcs/SvnDriver.php b/src/Composer/Repository/Vcs/SvnDriver.php index a8f0c4ad4..216c886ef 100644 --- a/src/Composer/Repository/Vcs/SvnDriver.php +++ b/src/Composer/Repository/Vcs/SvnDriver.php @@ -310,7 +310,7 @@ class SvnDriver extends VcsDriver $processExecutor = new ProcessExecutor($io); $exit = $processExecutor->execute( - "svn info --non-interactive ".ProcessExecutor::escape($url), + "svn info --non-interactive -- ".ProcessExecutor::escape($url), $ignoredOutput ); diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index cd714c7e8..4f59839a9 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -254,7 +254,7 @@ class Git $commandCallable = function ($url) { $sanitizedUrl = preg_replace('{://([^@]+?):(.+?)@}', '://', $url); - return sprintf('git remote set-url origin %s && git remote update --prune origin && git remote set-url origin %s', ProcessExecutor::escape($url), ProcessExecutor::escape($sanitizedUrl)); + return sprintf('git remote set-url origin -- %s && git remote update --prune origin && git remote set-url origin -- %s', ProcessExecutor::escape($url), ProcessExecutor::escape($sanitizedUrl)); }; $this->runCommand($commandCallable, $url, $dir); } catch (\Exception $e) { @@ -270,7 +270,7 @@ class Git $this->filesystem->removeDirectory($dir); $commandCallable = function ($url) use ($dir) { - return sprintf('git clone --mirror %s %s', ProcessExecutor::escape($url), ProcessExecutor::escape($dir)); + return sprintf('git clone --mirror -- %s %s', ProcessExecutor::escape($url), ProcessExecutor::escape($dir)); }; $this->runCommand($commandCallable, $url, $dir, true); diff --git a/src/Composer/Util/Svn.php b/src/Composer/Util/Svn.php index 58114ac93..80c420feb 100644 --- a/src/Composer/Util/Svn.php +++ b/src/Composer/Util/Svn.php @@ -224,7 +224,7 @@ class Svn protected function getCommand($cmd, $url, $path = null) { $cmd = sprintf( - '%s %s%s %s', + '%s %s%s -- %s', $cmd, '--non-interactive ', $this->getCredentialString(), diff --git a/tests/Composer/Test/Downloader/FossilDownloaderTest.php b/tests/Composer/Test/Downloader/FossilDownloaderTest.php index 97706435d..ab8e0ab71 100644 --- a/tests/Composer/Test/Downloader/FossilDownloaderTest.php +++ b/tests/Composer/Test/Downloader/FossilDownloaderTest.php @@ -70,19 +70,19 @@ class FossilDownloaderTest extends TestCase ->will($this->returnValue(array('http://fossil.kd2.org/kd2fw/'))); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); - $expectedFossilCommand = $this->getCmd('fossil clone \'http://fossil.kd2.org/kd2fw/\' \'repo.fossil\''); + $expectedFossilCommand = $this->getCmd('fossil clone -- \'http://fossil.kd2.org/kd2fw/\' \'repo.fossil\''); $processExecutor->expects($this->at(0)) ->method('execute') ->with($this->equalTo($expectedFossilCommand)) ->will($this->returnValue(0)); - $expectedFossilCommand = $this->getCmd('fossil open \'repo.fossil\' --nested'); + $expectedFossilCommand = $this->getCmd('fossil open --nested -- \'repo.fossil\''); $processExecutor->expects($this->at(1)) ->method('execute') ->with($this->equalTo($expectedFossilCommand)) ->will($this->returnValue(0)); - $expectedFossilCommand = $this->getCmd('fossil update \'trunk\''); + $expectedFossilCommand = $this->getCmd('fossil update -- \'trunk\''); $processExecutor->expects($this->at(2)) ->method('execute') ->with($this->equalTo($expectedFossilCommand)) diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index 5bf75d4f6..cd446a73b 100644 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -108,7 +108,7 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue('dev-master')); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://example.com/composer/composer' && git fetch composer && git remote set-url origin 'https://example.com/composer/composer' && git remote set-url composer 'https://example.com/composer/composer'"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout -- 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer -- 'https://example.com/composer/composer' && git fetch composer && git remote set-url origin -- 'https://example.com/composer/composer' && git remote set-url composer -- 'https://example.com/composer/composer'"); $processExecutor->expects($this->at(0)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -159,7 +159,7 @@ class GitDownloaderTest extends TestCase $filesystem = new \Composer\Util\Filesystem; $filesystem->removeDirectory($cachePath); - $expectedGitCommand = $this->winCompat(sprintf("git clone --mirror 'https://example.com/composer/composer' '%s'", $cachePath)); + $expectedGitCommand = $this->winCompat(sprintf("git clone --mirror -- 'https://example.com/composer/composer' '%s'", $cachePath)); $processExecutor->expects($this->at(0)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -181,7 +181,7 @@ class GitDownloaderTest extends TestCase ->with($this->equalTo($this->winCompat('git rev-parse --quiet --verify \'1234567890123456789012345678901234567890^{commit}\'')), $this->equalTo(null), $this->equalTo($this->winCompat($cachePath))) ->will($this->returnValue(0)); - $expectedGitCommand = $this->winCompat(sprintf("git clone --no-checkout '%1\$s' 'composerPath' --dissociate --reference '%1\$s' && cd 'composerPath' && git remote set-url origin 'https://example.com/composer/composer' && git remote add composer 'https://example.com/composer/composer'", $cachePath)); + $expectedGitCommand = $this->winCompat(sprintf("git clone --no-checkout '%1\$s' 'composerPath' --dissociate --reference '%1\$s' && cd 'composerPath' && git remote set-url origin -- 'https://example.com/composer/composer' && git remote add composer -- 'https://example.com/composer/composer'", $cachePath)); $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -224,7 +224,7 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue('1.0.0')); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://github.com/mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://github.com/mirrors/composer' && git fetch composer && git remote set-url origin 'https://github.com/mirrors/composer' && git remote set-url composer 'https://github.com/mirrors/composer'"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout -- 'https://github.com/mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer -- 'https://github.com/mirrors/composer' && git fetch composer && git remote set-url origin -- 'https://github.com/mirrors/composer' && git remote set-url composer -- 'https://github.com/mirrors/composer'"); $processExecutor->expects($this->at(0)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -235,19 +235,19 @@ class GitDownloaderTest extends TestCase ->with() ->will($this->returnValue('Error1')); - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'git@github.com:mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'git@github.com:mirrors/composer' && git fetch composer && git remote set-url origin 'git@github.com:mirrors/composer' && git remote set-url composer 'git@github.com:mirrors/composer'"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout -- 'git@github.com:mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer -- 'git@github.com:mirrors/composer' && git fetch composer && git remote set-url origin -- 'git@github.com:mirrors/composer' && git remote set-url composer -- 'git@github.com:mirrors/composer'"); $processExecutor->expects($this->at(2)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) ->will($this->returnValue(0)); - $expectedGitCommand = $this->winCompat("git remote set-url origin 'https://github.com/composer/composer'"); + $expectedGitCommand = $this->winCompat("git remote set-url origin -- 'https://github.com/composer/composer'"); $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($expectedGitCommand), $this->equalTo(null), $this->equalTo($this->winCompat('composerPath'))) ->will($this->returnValue(0)); - $expectedGitCommand = $this->winCompat("git remote set-url --push origin 'git@github.com:composer/composer.git'"); + $expectedGitCommand = $this->winCompat("git remote set-url --push origin -- 'git@github.com:composer/composer.git'"); $processExecutor->expects($this->at(4)) ->method('execute') ->with($this->equalTo($expectedGitCommand), $this->equalTo(null), $this->equalTo($this->winCompat('composerPath'))) @@ -299,13 +299,13 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue('1.0.0')); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); - $expectedGitCommand = $this->winCompat("git clone --no-checkout '{$url}' 'composerPath' && cd 'composerPath' && git remote add composer '{$url}' && git fetch composer && git remote set-url origin '{$url}' && git remote set-url composer '{$url}'"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout -- '{$url}' 'composerPath' && cd 'composerPath' && git remote add composer -- '{$url}' && git fetch composer && git remote set-url origin -- '{$url}' && git remote set-url composer -- '{$url}'"); $processExecutor->expects($this->at(0)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) ->will($this->returnValue(0)); - $expectedGitCommand = $this->winCompat("git remote set-url --push origin '{$pushUrl}'"); + $expectedGitCommand = $this->winCompat("git remote set-url --push origin -- '{$pushUrl}'"); $processExecutor->expects($this->at(1)) ->method('execute') ->with($this->equalTo($expectedGitCommand), $this->equalTo(null), $this->equalTo($this->winCompat('composerPath'))) @@ -327,7 +327,7 @@ class GitDownloaderTest extends TestCase */ public function testDownloadThrowsRuntimeExceptionIfGitCommandFails() { - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://example.com/composer/composer' && git fetch composer && git remote set-url origin 'https://example.com/composer/composer' && git remote set-url composer 'https://example.com/composer/composer'"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout -- 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer -- 'https://example.com/composer/composer' && git fetch composer && git remote set-url origin -- 'https://example.com/composer/composer' && git remote set-url composer -- 'https://example.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) ->method('getSourceReference') @@ -362,7 +362,7 @@ class GitDownloaderTest extends TestCase public function testUpdate() { - $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer 'https://github.com/composer/composer'"); + $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer -- 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- 'https://github.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) @@ -411,7 +411,7 @@ class GitDownloaderTest extends TestCase public function testUpdateWithNewRepoUrl() { - $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer 'https://github.com/composer/composer'"); + $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer -- 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- 'https://github.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) @@ -465,11 +465,11 @@ composer https://github.com/old/url (push) ->will($this->returnValue(0)); $processExecutor->expects($this->at(7)) ->method('execute') - ->with($this->equalTo($this->winCompat("git remote set-url origin 'https://github.com/composer/composer'")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) + ->with($this->equalTo($this->winCompat("git remote set-url origin -- 'https://github.com/composer/composer'")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(8)) ->method('execute') - ->with($this->equalTo($this->winCompat("git remote set-url --push origin 'git@github.com:composer/composer.git'")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) + ->with($this->equalTo($this->winCompat("git remote set-url --push origin -- 'git@github.com:composer/composer.git'")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) ->will($this->returnValue(0)); $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); @@ -483,7 +483,7 @@ composer https://github.com/old/url (push) */ public function testUpdateThrowsRuntimeExceptionIfGitCommandFails() { - $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer 'https://github.com/composer/composer'"); + $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer -- 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- 'https://github.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) @@ -524,8 +524,8 @@ composer https://github.com/old/url (push) public function testUpdateDoesntThrowsRuntimeExceptionIfGitCommandFailsAtFirstButIsAbleToRecover() { - $expectedFirstGitUpdateCommand = $this->winCompat("(git remote set-url composer '' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer ''"); - $expectedSecondGitUpdateCommand = $this->winCompat("(git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer 'https://github.com/composer/composer'"); + $expectedFirstGitUpdateCommand = $this->winCompat("(git remote set-url composer -- '' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- ''"); + $expectedSecondGitUpdateCommand = $this->winCompat("(git remote set-url composer -- 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- 'https://github.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) diff --git a/tests/Composer/Test/Downloader/HgDownloaderTest.php b/tests/Composer/Test/Downloader/HgDownloaderTest.php index c074e18dd..34363d29a 100644 --- a/tests/Composer/Test/Downloader/HgDownloaderTest.php +++ b/tests/Composer/Test/Downloader/HgDownloaderTest.php @@ -70,13 +70,13 @@ class HgDownloaderTest extends TestCase ->will($this->returnValue(array('https://mercurial.dev/l3l0/composer'))); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); - $expectedGitCommand = $this->getCmd('hg clone \'https://mercurial.dev/l3l0/composer\' \'composerPath\''); + $expectedGitCommand = $this->getCmd('hg clone -- \'https://mercurial.dev/l3l0/composer\' \'composerPath\''); $processExecutor->expects($this->at(0)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) ->will($this->returnValue(0)); - $expectedGitCommand = $this->getCmd('hg up \'ref\''); + $expectedGitCommand = $this->getCmd('hg up -- \'ref\''); $processExecutor->expects($this->at(1)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -122,7 +122,7 @@ class HgDownloaderTest extends TestCase ->method('execute') ->with($this->equalTo($expectedHgCommand)) ->will($this->returnValue(0)); - $expectedHgCommand = $this->getCmd("hg pull 'https://github.com/l3l0/composer' && hg up 'ref'"); + $expectedHgCommand = $this->getCmd("hg pull -- 'https://github.com/l3l0/composer' && hg up -- 'ref'"); $processExecutor->expects($this->at(1)) ->method('execute') ->with($this->equalTo($expectedHgCommand)) diff --git a/tests/Composer/Test/Util/SvnTest.php b/tests/Composer/Test/Util/SvnTest.php index 03a183583..ada3818ae 100644 --- a/tests/Composer/Test/Util/SvnTest.php +++ b/tests/Composer/Test/Util/SvnTest.php @@ -60,7 +60,7 @@ class SvnTest extends TestCase $reflMethod->setAccessible(true); $this->assertEquals( - $this->getCmd("svn ls --non-interactive 'http://svn.example.org'"), + $this->getCmd("svn ls --non-interactive -- 'http://svn.example.org'"), $reflMethod->invokeArgs($svn, array('svn ls', $url)) ); } From 1cdbacbe07eac7d360c6bf27931da715b008b84e Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 27 Apr 2021 13:03:36 +0200 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 844f1b5e9..bc6a92fc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### [1.10.22] 2021-04-27 + + * Security: Fixed command injection vulnerability in HgDriver/HgDownloader and hardened other VCS drivers and downloaders (GHSA-h5h8-pc6h-jvvx / CVE-2021-29472) + ### [1.10.21] 2021-04-01 * Fixed support for new GitHub OAuth token format @@ -934,6 +938,7 @@ * Initial release +[1.10.22]: https://github.com/composer/composer/compare/1.10.21...1.10.22 [1.10.21]: https://github.com/composer/composer/compare/1.10.20...1.10.21 [1.10.20]: https://github.com/composer/composer/compare/1.10.19...1.10.20 [1.10.19]: https://github.com/composer/composer/compare/1.10.18...1.10.19