8000 Merge pull request from GHSA-x7cr-6qr6-2hh6 · composer/composer@c33aafa · GitHub
[go: up one dir, main page]

Skip to content

Commit c33aafa

Browse files
glaubinixSeldaek
authored andcommitted
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
1 parent d4b29e9 commit c33aafa

File tree

4 files changed

+157
-4
lines changed

4 files changed

+157
-4
lines changed

src/Composer/Repository/Vcs/GitDriver.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ public function getDist($identifier)
130130
*/
131131
public function getFileContent($file, $identifier)
132132
{
133+
if (isset($identifier[0]) && $identifier[0] === '-') {
134+
throw new \RuntimeException('Invalid git identifier detected. Identifier must not start with a -, given: ' . $identifier);
135+
}
136+
133137
$resource = sprintf('%s:%s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file));
134138
$this->process->execute(sprintf('git show %s', $resource), $content, $this->repoDir);
135139

@@ -183,7 +187,7 @@ public function getBranches()
183187
$this->process->execute('git branch --no-color --no-abbrev -v', $output, $this->repoDir);
184188
foreach ($this->process->splitLines($output) as $branch) {
185189
if ($branch && !preg_match('{^ *[^/]+/HEAD }', $branch)) {
186-
if (preg_match('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match)) {
190+
if (preg_match('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match) && $match[1][0] !== '-') {
187191
$branches[$match[1]] = $match[2];
188192
}
189193
}

src/Composer/Repository/Vcs/HgDriver.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ public function getDist($identifier)
122122
*/
123123
public function getFileContent($file, $identifier)
124124
{
125-
$resource = sprintf('hg cat -r %s %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file));
125+
if (isset($identifier[0]) && $identifier[0] === '-') {
126+
throw new \RuntimeException('Invalid hg identifier detected. Identifier must not start with a -, given: ' . $identifier);
127+
}
128+
129+
$resource = sprintf('hg cat -r %s -- %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file));
126130
$this->process->execute($resource, $content, $this->repoDir);
127131

128132
if (!trim($content)) {
@@ -182,14 +186,14 @@ public function getBranches()
182186

183187
$this->process->execute('hg branches', $output, $this->repoDir);
184188
foreach ($this->process->splitLines($output) as $branch) {
185-
if ($branch && preg_match('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match)) {
189+
if ($branch && preg_match('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match) && $match[1][0] !== '-') {
186190
$branches[$match[1]] = $match[2];
187191
}
188192
}
189193

190194
$this->process->execute('hg bookmarks', $output, $this->repoDir);
191195
foreach ($this->process->splitLines($output) as $branch) {
192-
if ($branch && preg_match('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match)) {
196+
if ($branch && preg_match('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match) && $match[1][0] !== '-') {
193197
$bookmarks[$match[1]] = $match[2];
194198
}
195199
}
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
<?php
2+
3+
namespace Composer\Test\Repository\Vcs;
4+
5+
use Composer\Config;
6+
use Composer\Repository\Vcs\GitDriver;
7+
use Composer\Test\Mock\ProcessExecutorMock;
8+
use Composer\Test\TestCase;
9+
10+
class GitDriverTest extends TestCase
11+
{
12+
/** @var Config */
13+
private $config;
14+
/** @var string */
15+
private $home;
16+
17+
public function setUp()
18+
{
19+
$this->home = self::getUniqueTmpDirectory();
20+
$this->config = new Config();
21+
$this->config->merge(array(
22+
'config' => array(
23+
'home' => $this->home,
24+
),
25+
));
26+
}
27+
28+
public function testGetBranchesFilterInvalidBranchNames()
29+
{
30+
$process = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock();
31+
$process->expects($this->any())
32+
->method('execute')
33+
->will($this->returnValue(0));
34+
$io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
35+
36+
$driver = new GitDriver(array('url' => 'https://example.org/acme.git'), $io, $this->config, $process);
37+
$this->setRepoDir($driver, $this->home);
38+
39+
// Branches starting with a - character are not valid git branches names
40+
// Still assert that they get filtered to prevent issues later on
41+
$stdout = <<<GIT
42+
* main 089681446ba44d6d9004350192486f2ceb4eaa06 commit
43+
2.2 12681446ba44d6d9004350192486f2ceb4eaa06 commit
44+
-h 089681446ba44d6d9004350192486f2ceb4eaa06 commit
45+
GIT;
46+
47+
$process->expects($this->at(0))
48+
->method('execute')
49+
->with('git branch --no-color --no-abbrev -v');
50+
$process->expects($this->at(1))
51+
->method('splitLines')
52+
->will($this->returnValue(preg_split('{\r?\n}', trim($stdout))));
53+
54+
$branches = $driver->getBranches();
55+
$this->assertSame(array(
56+
'main' => '089681446ba44d6d9004350192486f2ceb4eaa06',
57+
'2.2' => '12681446ba44d6d9004350192486f2ceb4eaa06',
58+
), $branches);
59+
}
60+
61+
public function testFileGetContentInvalidIdentifier()
62+
{
63+
$this->setExpectedException('\RuntimeException');
64+
65+
$process = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock();
66+
$process->expects($this->any())
67+
->method('execute')
68+
->will($this->returnValue(0));
69+
$io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
70+
$driver = new GitDriver(array('url' => 'https://example.org/acme.git'), $io, $this->config, $process);
71+
72+
$this->assertNull($driver->getFileContent('file.txt', 'h'));
73+
74+
$driver->getFileContent('file.txt', '-h');
75+
}
76+
77+
/**
78+
* @param GitDriver $driver
79+
* @param string $path
80+
*/
81+
private function setRepoDir($driver, $path)
82+
{
83+
$reflectionClass = new \ReflectionClass($driver);
84+
$reflectionProperty = $reflectionClass->getProperty('repoDir');
85+
$reflectionProperty->setAccessible(true);
86+
$reflectionProperty->setValue($driver, $path);
87+
}
88+
}

tests/Composer/Test/Repository/Vcs/HgDriverTest.php

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313
namespace Composer\Test\Repository\Vcs;
1414

1515
use Composer\Repository\Vcs\HgDriver;
16+
use Composer\Test\Mock\ProcessExecutorMock;
1617
use Composer\Test\TestCase;
1718
use Composer\Util\Filesystem;
1819
use Composer\Config;
20+
use Composer\Util\ProcessExecutor;
1921

2022
class HgDriverTest extends TestCase
2123
{
@@ -64,4 +66,59 @@ public function supportsDataProvider()
6466
array('https://user@bitbucket.org/user/repo'),
6567
);
6668
}
69+
70+
public function testGetBranchesFilterInvalidBranchNames()
71+
{
72+
$process = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock();
73+
$process->expects($this->any())
74+
->method('execute')
75+
->will($this->returnValue(0));
76+
77+
$driver = new HgDriver(array('url' => 'https://example.org/acme.git'), $this->io, $this->config, $process);
78+
79+
$stdout = <<<HG_BRANCHES
80+
default 1:dbf6c8acb640
81+
--help 1:dbf6c8acb640
82+
HG_BRANCHES;
83+
84+
$stdout1 = <<<HG_BOOKMARKS
85+
help 1:dbf6c8acb641
86+
--help 1:dbf6c8acb641
87+
88+
HG_BOOKMARKS;
89+
90+
$process->expects($this->at(0))
91+
->method('execute')
92+
->with('hg branches');
93+
$process->expects($this->at(1))
94+
->method('splitLines')
95+
->will($this->returnValue(preg_split('{\r?\n}', trim($stdout))));
96+
$process->expects($this->at(2))
97+
->method('execute')
98+
->with('hg bookmarks');
99+
$process->expects($this->at(3))
100+
->method('splitLines')
101+
->will($this->returnValue(preg_split('{\r?\n}', trim($stdout1))));
102+
103+
$branches = $driver->getBranches();
104+
$this->assertSame(array(
105+
'help' => 'dbf6c8acb641',
106+
'default' => 'dbf6c8acb640',
107+
), $branches);
108+
}
109+
110+
public function testFileGetContentInvalidIdentifier()
111+
{
112+
$this->setExpectedException('\RuntimeException');
113+
114+
$process = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock();
115+
$process->expects($this->any())
116+
->method('execute')
117+
->will($this->returnValue(0));
118+
$driver = new HgDriver(array('url' => 'https://example.org/acme.git'), $this->io, $this->config, $process);
119+
120+
$this->assertNull($driver->getFileContent('file.txt', 'h'));
121+
122+
$driver->getFileContent('file.txt', '-h');
123+
}
67124
}

0 commit comments

Comments
 (0)
0