From 2c40c53637c5c7e43fff7c09d3d324d632734709 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 | 81 +++++++++++++++++++ .../Test/Repository/Vcs/HgDriverTest.php | 46 +++++++++++ 4 files changed, 139 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 b91508acd..039f9052f 100644 --- a/src/Composer/Repository/Vcs/GitDriver.php +++ b/src/Composer/Repository/Vcs/GitDriver.php @@ -138,6 +138,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); @@ -191,7 +195,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 73bc426e5..f9f8064ba 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($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)) { @@ -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/tests/Composer/Test/Repository/Vcs/GitDriverTest.php b/tests/Composer/Test/Repository/Vcs/GitDriverTest.php new file mode 100644 index 000000000..c8cfdab0a --- /dev/null +++ b/tests/Composer/Test/Repository/Vcs/GitDriverTest.php @@ -0,0 +1,81 @@ +home = self::getUniqueTmpDirectory(); + $this->config = new Config(); + $this->config->merge(array( + 'config' => array( + 'home' => $this->home, + ), + )); + } + + public function testGetBranchesFilterInvalidBranchNames() + { + $process = new ProcessExecutorMock; + $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() + { + $this->expectException('\RuntimeException'); + + $process = new ProcessExecutorMock; + $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'); + } + + /** + * @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 2fcd27bdf..356a28c48 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; @@ -66,4 +67,49 @@ class HgDriverTest extends TestCase array('https://user@bitbucket.org/user/repo'), ); } + + public function testGetBranchesFilterInvalidBranchNames() + { + $process = new ProcessExecutorMock; + + $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() + { + $this->expectException('\RuntimeException'); + + $process = new ProcessExecutorMock; + $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'); + } }