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
composer · Apr 13, 2022 · c33aafa · c33aafa
1 parent d4b29e9
commit c33aafa
Show file tree

Hide file tree

Showing 4 changed files with 157 additions and 4 deletions.
diff --git a/src/Composer/Repository/Vcs/GitDriver.php b/src/Composer/Repository/Vcs/GitDriver.php
@@ -130,6 +130,10 @@ public function getDist($identifier)
      */
     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 @@ public function getBranches()
             $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
@@ -122,7 +122,11 @@ public function getDist($identifier)
      */
     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 @@ public function getBranches()
 
             $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
@@ -0,0 +1,88 @@
+<?php
+
+namespace Composer\Test\Repository\Vcs;
+
+use Composer\Config;
+use Composer\Repository\Vcs\GitDriver;
+use Composer\Test\Mock\ProcessExecutorMock;
+use Composer\Test\TestCase;
+
+class GitDriverTest extends TestCase
+{
+    /** @var Config */
+    private $config;
+    /** @var string */
+    private $home;
+
+    public function setUp()
+    {
+        $this->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 = <<<GIT
+* main 089681446ba44d6d9004350192486f2ceb4eaa06 commit
+  2.2  12681446ba44d6d9004350192486f2ceb4eaa06 commit
+  -h   089681446ba44d6d9004350192486f2ceb4eaa06 commit
+GIT;
+
+        $process->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
@@ -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 @@ public function supportsDataProvider()
             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 = <<<HG_BRANCHES
+default 1:dbf6c8acb640
+--help  1:dbf6c8acb640
+HG_BRANCHES;
+
+        $stdout1 = <<<HG_BOOKMARKS
+help    1:dbf6c8acb641
+--help  1:dbf6c8acb641
+
+HG_BOOKMARKS;
+
+        $process->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');
+    }
 }