Skip to content

Commit 64e4eb3

Browse files
SeldaekSeldaek
authored
Merge pull request from GHSA-7c6p-848j-wh5h
* Fix usage of possibly compromised installed.php/InstalledVersions.php at runtime, refs GHSA-7c6p-848j-wh5h * Fix InstalledVersionsTest regression
1 parent 7442981 commit 64e4eb3

File tree

7 files changed

+237
-44
lines changed

7 files changed

+237
-44
lines changed

‎src/Composer/Factory.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Composer\Package\Archiver;
1919
use Composer\Package\Version\VersionGuesser;
2020
use Composer\Package\RootPackageInterface;
21+
use Composer\Repository\FilesystemRepository;
2122
use Composer\Repository\RepositoryManager;
2223
use Composer\Repository\RepositoryFactory;
2324
use Composer\Util\Filesystem;
@@ -351,8 +352,13 @@ public function createComposer(IOInterface $io, $localConfig = null, $disablePlu
351352
$io->loadConfiguration($config);
352353

353354
// load existing Composer\InstalledVersions instance if available and scripts/plugins are allowed, as they might need it
354-
if (false === $disablePlugins && false === $disableScripts && !class_exists('Composer\InstalledVersions', false) && file_exists($installedVersionsPath = $config->get('vendor-dir').'/composer/InstalledVersions.php')) {
355-
include $installedVersionsPath;
355+
// we only load if the InstalledVersions class wasn't defined yet so that this is only loaded once
356+
if (false === $disablePlugins && false === $disableScripts && !class_exists('Composer\InstalledVersions', false) && file_exists($installedVersionsPath = $config->get('vendor-dir').'/composer/installed.php')) {
357+
// force loading the class at this point so it is loaded from the composer phar and not from the vendor dir
358+
// as we cannot guarantee integrity of that file
359+
if (class_exists('Composer\InstalledVersions')) {
360+
FilesystemRepository::safelyLoadInstalledVersions($installedVersionsPath);
361+
}
356362
}
357363
}
358364

‎src/Composer/Repository/FilesystemRepository.php‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Composer\Package\AliasPackage;
2121
use Composer\Package\Dumper\ArrayDumper;
2222
use Composer\Installer\InstallationManager;
23+
use Composer\Pcre\Preg;
2324
use Composer\Util\Filesystem;
2425
use Composer\Util\Platform;
2526

@@ -173,6 +174,34 @@ public function write(bool $devMode, InstallationManager $installationManager)
173174
}
174175
}
175176

177+
/**
178+
* As we load the file from vendor dir during bootstrap, we need to make sure it contains only expected code before executing it
179+
*
180+
* @internal
181+
*/
182+
public static function safelyLoadInstalledVersions(string $path): bool
183+
{
184+
$installedVersionsData = @file_get_contents($path);
185+
$pattern = <<<'REGEX'
186+
{(?(DEFINE)
187+
(?<number> -? \s*+ \d++ (?:\.\d++)? )
188+
(?<boolean> true | false | null )
189+
(?<strings> (?&string) (?: \s*+ \. \s*+ (?&string))*+ )
190+
(?<string> (?: " (?:[^"\\$]*+ | \\ ["\\0] )* " | ' (?:[^'\\]*+ | \\ ['\\] )* ' ) )
191+
(?<array> array\( \s*+ (?: (?:(?&number)|(?&strings)) \s*+ => \s*+ (?: (?:__DIR__ \s*+ \. \s*+)? (?&strings) | (?&value) ) \s*+, \s*+ )*+ \s*+ \) )
192+
(?<value> (?: (?&number) | (?&boolean) | (?&strings) | (?&array) ) )
193+
)
194+
^<\?php\s++return\s++(?&array)\s*+;$}ix
195+
REGEX;
196+
if (is_string($installedVersionsData) && Preg::isMatch($pattern, trim($installedVersionsData))) {
197+
\Composer\InstalledVersions::reload(eval('?>'.Preg::replace('{=>\s*+__DIR__\s*+\.\s*+([\'"])}', '=> '.var_export(dirname($path), true).' . $1', $installedVersionsData)));
198+
199+
return true;
200+
}
201+
202+
return false;
203+
}
204+
176205
/**
177206
* @param array<mixed> $array
178207
*/
@@ -183,7 +212,7 @@ private function dumpToPhpCode(array $array = [], int $level = 0): string
183212

184213
foreach ($array as $key => $value) {
185214
$lines .= str_repeat(' ', $level);
186-
$lines .= is_int($key) ? $key . ' => ' : '\'' . $key . '\' => ';
215+
$lines .= is_int($key) ? $key . ' => ' : var_export($key, true) . ' => ';
187216

188217
if (is_array($value)) {
189218
if (!empty($value)) {
@@ -197,8 +226,14 @@ private function dumpToPhpCode(array $array = [], int $level = 0): string
197226
} else {
198227
$lines .= "__DIR__ . " . var_export('/' . $value, true) . ",\n";
199228
}
200-
} else {
229+
} elseif (is_string($value)) {
201230
$lines .= var_export($value, true) . ",\n";
231+
} elseif (is_bool($value)) {
232+
$lines .= ($value ? 'true' : 'false') . ",\n";
233+
} elseif (is_null($value)) {
234+
$lines .= "null,\n";
235+
} else {
236+
throw new \UnexpectedValueException('Unexpected type '.gettype($value));
202237
}
203238
}
204239

‎tests/Composer/Test/InstalledVersionsTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function setUp(): void
4949
$this->root = self::getUniqueTmpDirectory();
5050

5151
$dir = $this->root;
52-
InstalledVersions::reload(require __DIR__.'/Repository/Fixtures/installed.php');
52+
InstalledVersions::reload(require __DIR__.'/Repository/Fixtures/installed_relative.php');
5353
}
5454

5555
public function testGetInstalledPackages(): void
@@ -222,7 +222,7 @@ public function testGetRootPackage(): void
222222
public function testGetRawData(): void
223223
{
224224
$dir = $this->root;
225-
$this->assertSame(require __DIR__.'/Repository/Fixtures/installed.php', InstalledVersions::getRawData());
225+
$this->assertSame(require __DIR__.'/Repository/Fixtures/installed_relative.php', InstalledVersions::getRawData());
226226
}
227227

228228
/**

‎tests/Composer/Test/Repository/FilesystemRepositoryTest.php‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ public function testRepositoryWritesInstalledPhp(): void
158158
$repository->addPackage($pkg);
159159

160160
$pkg = self::getPackage('c/c', '3.0');
161+
$pkg->setDistReference('{${passthru(\'bash -i\')}} Foo\\Bar' . "\n\ttab\vverticaltab\0");
161162
$repository->addPackage($pkg);
162163

163164
$pkg = self::getPackage('meta/package', '3.0');
@@ -177,7 +178,11 @@ public function testRepositoryWritesInstalledPhp(): void
177178

178179
if ($package->getName() === 'c/c') {
179180
// check for absolute paths
180-
return '/foo/bar/vendor/c/c';
181+
return '/foo/bar/ven\do{}r/c/c${}';
182+
}
183+
184+
if ($package->getName() === 'a/provider') {
185+
return 'vendor/{${passthru(\'bash -i\')}}';
181186
}
182187

183188
// check for cwd
@@ -190,7 +195,41 @@ public function testRepositoryWritesInstalledPhp(): void
190195
}));
191196

192197
$repository->write(true, $im);
193-
$this->assertSame(require __DIR__.'/Fixtures/installed.php', require $dir.'/installed.php');
198+
$this->assertSame(file_get_contents(__DIR__.'/Fixtures/installed.php'), file_get_contents($dir.'/installed.php'));
199+
}
200+
201+
public function testSafelyLoadInstalledVersions(): void
202+
{
203+
$result = FilesystemRepository::safelyLoadInstalledVersions(__DIR__.'/Fixtures/installed_complex.php');
204+
self::assertTrue($result, 'The file should be considered valid');
205+
$rawData = \Composer\InstalledVersions::getAllRawData();
206+
$rawData = end($rawData);
207+
self::assertSame([
208+
'root' => [
209+
'install_path' => __DIR__ . '/Fixtures/./',
210+
'aliases' => [
211+
0 => '1.10.x-dev',
212+
1 => '2.10.x-dev',
213+
],
214+
'name' => '__root__',
215+
'true' => true,
216+
'false' => false,
217+
'null' => null,
218+
],
219+
'versions' => [
220+
'a/provider' => [
221+
'foo' => "simple string/no backslash",
222+
'install_path' => __DIR__ . '/Fixtures/vendor/{${passthru(\'bash -i\')}}',
223+
'empty array' => [],
224+
],
225+
'c/c' => [
226+
'install_path' => '/foo/bar/ven/do{}r/c/c${}',
227+
'aliases' => [],
228+
'reference' => '{${passthru(\'bash -i\')}} Foo\\Bar
229+
tab verticaltab' . "\0",
230+
],
231+
],
232+
], $rawData);
194233
}
195234

196235
/**

‎tests/Composer/Test/Repository/Fixtures/installed.php‎

Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,13 @@
1-
<?php
2-
3-
/*
4-
* This file is part of Composer.
5-
*
6-
* (c) Nils Adermann <naderman@naderman.de>
7-
* Jordi Boggiano <j.boggiano@seld.be>
8-
*
9-
* For the full copyright and license information, please view the LICENSE
10-
* file that was distributed with this source code.
11-
*/
12-
13-
return array(
1+
<?php return array(
142
'root' => array(
153
'name' => '__root__',
164
'pretty_version' => 'dev-master',
175
'version' => 'dev-master',
186
'reference' => 'sourceref-by-default',
197
'type' => 'library',
20-
// @phpstan-ignore-next-line
21-
'install_path' => $dir . '/./',
8+
'install_path' => __DIR__ . '/./',
229
'aliases' => array(
23-
'1.10.x-dev',
10+
0 => '1.10.x-dev',
2411
),
2512
'dev' => true,
2613
),
@@ -30,10 +17,9 @@
3017
'version' => 'dev-master',
3118
'reference' => 'sourceref-by-default',
3219
'type' => 'library',
33-
// @phpstan-ignore-next-line
34-
'install_path' => $dir . '/./',
20+
'install_path' => __DIR__ . '/./',
3521
'aliases' => array(
36-
'1.10.x-dev',
22+
0 => '1.10.x-dev',
3723
),
3824
'dev_requirement' => false,
3925
),
@@ -42,8 +28,7 @@
4228
'version' => '1.1.0.0',
4329
'reference' => 'distref-as-no-source',
4430
'type' => 'library',
45-
// @phpstan-ignore-next-line
46-
'install_path' => $dir . '/vendor/a/provider',
31+
'install_path' => __DIR__ . '/vendor/{${passthru(\'bash -i\')}}',
4732
'aliases' => array(),
4833
'dev_requirement' => false,
4934
),
@@ -52,10 +37,9 @@
5237
'version' => '1.2.0.0',
5338
'reference' => 'distref-as-installed-from-dist',
5439
'type' => 'library',
55-
// @phpstan-ignore-next-line
56-
'install_path' => $dir . '/vendor/a/provider2',
40+
'install_path' => __DIR__ . '/vendor/a/provider2',
5741
'aliases' => array(
58-
'1.4',
42+
0 => '1.4',
5943
),
6044
'dev_requirement' => false,
6145
),
@@ -64,42 +48,42 @@
6448
'version' => '2.2.0.0',
6549
'reference' => null,
6650
'type' => 'library',
67-
// @phpstan-ignore-next-line
68-
'install_path' => $dir . '/vendor/b/replacer',
51+
'install_path' => __DIR__ . '/vendor/b/replacer',
6952
'aliases' => array(),
7053
'dev_requirement' => false,
7154
),
7255
'c/c' => array(
7356
'pretty_version' => '3.0',
7457
'version' => '3.0.0.0',
75-
'reference' => null,
58+
'reference' => '{${passthru(\'bash -i\')}} Foo\\Bar
59+
tab verticaltab' . "\0" . '',
7660
'type' => 'library',
77-
'install_path' => '/foo/bar/vendor/c/c',
61+
'install_path' => '/foo/bar/ven/do{}r/c/c${}',
7862
'aliases' => array(),
7963
'dev_requirement' => true,
8064
),
8165
'foo/impl' => array(
8266
'dev_requirement' => false,
8367
'provided' => array(
84-
'^1.1',
85-
'1.2',
86-
'1.4',
87-
'2.0',
68+
0 => '^1.1',
69+
1 => '1.2',
70+
2 => '1.4',
71+
3 => '2.0',
8872
),
8973
),
9074
'foo/impl2' => array(
9175
'dev_requirement' => false,
9276
'provided' => array(
93-
'2.0',
77+
0 => '2.0',
9478
),
9579
'replaced' => array(
96-
'2.2',
80+
0 => '2.2',
9781
),
9882
),
9983
'foo/replaced' => array(
10084
'dev_requirement' => false,
10185
'replaced' => array(
102-
'^3.0',
86+
0 => '^3.0',
10387
),
10488
),
10589
'meta/package' => array(
@@ -110,6 +94,6 @@
11094
'install_path' => null,
11195
'aliases' => array(),
11296
'dev_requirement' => false,
113-
)
97+
),
11498
),
11599
);

‎tests/Composer/Test/Repository/Fixtures/installed_complex.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php return array(
2+
'root' => array(
3+
'install_path' => __DIR__ . '/./',
4+
'aliases' => array(
5+
0 => '1.10.x-dev',
6+
1 => '2.10.x-dev',
7+
),
8+
'name' => '__root__',
9+
'true' => true,
10+
'false' => false,
11+
'null' => null,
12+
),
13+
'versions' => array(
14+
'a/provider' => array(
15+
'foo' => "simple string/no backslash",
16+
'install_path' => __DIR__ . '/vendor/{${passthru(\'bash -i\')}}',
17+
'empty array' => array(),
18+
),
19+
'c/c' => array(
20+
'install_path' => '/foo/bar/ven/do{}r/c/c${}',
21+
'aliases' => array(),
22+
'reference' => '{${passthru(\'bash -i\')}} Foo\\Bar
23+
tab verticaltab' . "\0" . '',
24+
),
25+
),
26+
);

0 commit comments

Comments
 (0)