Skip to content

Commit

Permalink
Merge pull request from GHSA-x7cr-6qr6-2hh6
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
glaubinix committed Apr 13, 2022
1 parent 915b97f commit 2c40c53
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 4 deletions.
6 changes: 5 additions & 1 deletion src/Composer/Repository/Vcs/GitDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,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);

Expand Down Expand Up @@ -191,7 +195,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::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];
}
}
Expand Down
10 changes: 7 additions & 3 deletions src/Composer/Repository/Vcs/HgDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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)) {
Expand Down Expand Up @@ -186,14 +190,14 @@ public function getBranches()

$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];
}
}
Expand Down
81 changes: 81 additions & 0 deletions tests/Composer/Test/Repository/Vcs/GitDriverTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?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 = 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 = <<<GIT
* main 089681446ba44d6d9004350192486f2ceb4eaa06 commit
2.2 12681446ba44d6d9004350192486f2ceb4eaa06 commit
-h 089681446ba44d6d9004350192486f2ceb4eaa06 commit
GIT;

$process
->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);
}
}
46 changes: 46 additions & 0 deletions tests/Composer/Test/Repository/Vcs/HgDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -66,4 +67,49 @@ public function supportsDataProvider()
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 = <<<HG_BRANCHES
default 1:dbf6c8acb640
--help 1:dbf6c8acb640
HG_BRANCHES;

$stdout1 = <<<HG_BOOKMARKS
help 1:dbf6c8acb641
--help 1:dbf6c8acb641
HG_BOOKMARKS;

$process
->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');
}
}

0 comments on commit 2c40c53

Please sign in to comment.