From c33aafaa04114e4f20104cb4bff050f31abc6178 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 13 Apr 2022 14:54:58 +0100 Subject: [PATCH] Merge pull request from GHSA-x7cr-6qr6-2hh6 * GitDriver: filter branch names starting with a - character * GitDriver: getFileContent prevent identifiers starting with a - * HgDriver: prevent invalid identifiers and prevent file from running commands * HgDriver: filter branches starting with a - character --- src/Composer/Repository/Vcs/GitDriver.php | 6 +- src/Composer/Repository/Vcs/HgDriver.php | 10 ++- .../Test/Repository/Vcs/GitDriverTest.php | 88 +++++++++++++++++++ .../Test/Repository/Vcs/HgDriverTest.php | 57 ++++++++++++ 4 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 tests/Composer/Test/Repository/Vcs/GitDriverTest.php diff --git a/src/Composer/Repository/Vcs/GitDriver.php b/src/Composer/Repository/Vcs/GitDriver.php index 9e3fa23f0..c158a2e13 100644 --- a/src/Composer/Repository/Vcs/GitDriver.php +++ b/src/Composer/Repository/Vcs/GitDriver.php @@ -130,6 +130,10 @@ class GitDriver extends VcsDriver */ public function getFileContent($file, $identifier) { + 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); @@ -183,7 +187,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_match('{^ *[^/]+/HEAD }', $branch)) { - if (preg_match('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match)) { + if (preg_match('{^(?:\* )? *(\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 e739d4cda..d354caacb 100644 --- a/src/Composer/Repository/Vcs/HgDriver.php +++ b/src/Composer/Repository/Vcs/HgDriver.php @@ -122,7 +122,11 @@ class HgDriver extends VcsDriver */ public function getFileContent($file, $identifier) { - $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)) { @@ -182,14 +186,14 @@ class HgDriver extends VcsDriver $this->process->execute('hg branches', $output, $this->repoDir); foreach ($this->process->splitLines($output) as $branch) { - if ($branch && preg_match('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match)) { + if ($branch && preg_match('(^([^\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_match('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match)) { + if ($branch && preg_match('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match) && $match[1][0] !== '-') { $bookmarks[$match[1]] = $match[2]; } } diff --git a/tests/Composer/Test/Repository/Vcs/GitDriverTest.php b/tests/Composer/Test/Repository/Vcs/GitDriverTest.php new file mode 100644 index 000000000..429dc2bd7 --- /dev/null +++ b/tests/Composer/Test/Repository/Vcs/GitDriverTest.php @@ -0,0 +1,88 @@ +home = self::getUniqueTmpDirectory(); + $this->config = new Config(); + $this->config->merge(array( + 'config' => array( + 'home' => $this->home, + ), + )); + } + + public function testGetBranchesFilterInvalidBranchNames() + { + $process = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); + $process->expects($this->any()) + ->method('execute') + ->will($this->returnValue(0)); + $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(); + + $driver = new GitDriver(array('url' => 'https://example.org/acme.git'), $io, $this->config, $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($this->at(0)) + ->method('execute') + ->with('git branch --no-color --no-abbrev -v'); + $process->expects($this->at(1)) + ->method('splitLines') + ->will($this->returnValue(preg_split('{\r?\n}', trim($stdout)))); + + $branches = $driver->getBranches(); + $this->assertSame(array( + 'main' => '089681446ba44d6d9004350192486f2ceb4eaa06', + '2.2' => '12681446ba44d6d9004350192486f2ceb4eaa06', + ), $branches); + } + + public function testFileGetContentInvalidIdentifier() + { + $this->setExpectedException('\RuntimeException'); + + $process = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); + $process->expects($this->any()) + ->method('execute') + ->will($this->returnValue(0)); + $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(); + $driver = new GitDriver(array('url' => 'https://example.org/acme.git'), $io, $this->config, $process); + + $this->assertNull($driver->getFileContent('file.txt', 'h')); + + $driver->getFileContent('file.txt', '-h'); + } + + /** + * @param GitDriver $driver + * @param string $path + */ + private function setRepoDir($driver, $path) + { + $reflectionClass = new \ReflectionClass($driver); + $reflectionProperty = $reflectionClass->getProperty('repoDir'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($driver, $path); + } +} diff --git a/tests/Composer/Test/Repository/Vcs/HgDriverTest.php b/tests/Composer/Test/Repository/Vcs/HgDriverTest.php index 11143b476..143f58238 100644 --- a/tests/Composer/Test/Repository/Vcs/HgDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/HgDriverTest.php @@ -13,9 +13,11 @@ 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; +use Composer\Util\ProcessExecutor; class HgDriverTest extends TestCase { @@ -64,4 +66,59 @@ class HgDriverTest extends TestCase array('https://user@bitbucket.org/user/repo'), ); } + + public function testGetBranchesFilterInvalidBranchNames() + { + $process = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); + $process->expects($this->any()) + ->method('execute') + ->will($this->returnValue(0)); + + $driver = new HgDriver(array('url' => 'https://example.org/acme.git'), $this->io, $this->config, $process); + + $stdout = <<expects($this->at(0)) + ->method('execute') + ->with('hg branches'); + $process->expects($this->at(1)) + ->method('splitLines') + ->will($this->returnValue(preg_split('{\r?\n}', trim($stdout)))); + $process->expects($this->at(2)) + ->method('execute') + ->with('hg bookmarks'); + $process->expects($this->at(3)) + ->method('splitLines') + ->will($this->returnValue(preg_split('{\r?\n}', trim($stdout1)))); + + $branches = $driver->getBranches(); + $this->assertSame(array( + 'help' => 'dbf6c8acb641', + 'default' => 'dbf6c8acb640', + ), $branches); + } + + public function testFileGetContentInvalidIdentifier() + { + $this->setExpectedException('\RuntimeException'); + + $process = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); + $process->expects($this->any()) + ->method('execute') + ->will($this->returnValue(0)); + $driver = new HgDriver(array('url' => 'https://example.org/acme.git'), $this->io, $this->config, $process); + + $this->assertNull($driver->getFileContent('file.txt', 'h')); + + $driver->getFileContent('file.txt', '-h'); + } }