CVE-2022-24828 (PHP, published 2022-04-13T21:15Z)
Description: value: Composer is a dependency manager for the PHP programming language. Integrators using Composer code to call `VcsDriver::getFileContent` can have a code injection vulnerability if the user can control the `$file` or `$identifier` argument. This leads to a vulnerability on packagist.org for example where the composer.json's `readme` field can be used as a vector for injecting parameters into hg/Mercurial via the `$file` argument, or git via the `$identifier` argument if you allow arbitrary data there (Packagist does not, but maybe other integrators do). Composer itself should not be affected by the vulnerability as it does not call `getFileContent` with arbitrary data into `$file`/`$identifier`. To the best of our knowledge this was not abused, and the vulnerability has been patched on packagist.org and Private Packagist within a day of the vulnerability report.
Severity: MEDIUM (CVSS v3 base score: 8.8)
Weakness: CWE-88 \u2014 Improper Neutralization of Argument Delimiters in a Command ('Argument Injection')
The product constructs a string for a command to be executed by a separate component
in another control sphere, but it does not properly delimit the
intended arguments, options, or switches within that command string.
Repository: https://github.com/composer/composer
Files changed:
src/Composer/Repository/Vcs/GitDriver.php
src/Composer/Repository/Vcs/HgDriver.php
tests/Composer/Test/Repository/Vcs/GitDriverTest.php
tests/Composer/Test/Repository/Vcs/HgDriverTest.php
Commit message: 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

Vulnerable code:
                    if (Preg::isMatch('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match)) {
        $resource = sprintf('hg cat -r %s %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file));
                if ($branch && Preg::isMatch('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match)) {
                if ($branch && Preg::isMatch('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match)) {

Fixed code:
        if (isset($identifier[0]) && $identifier[0] === '-') {
            throw new \RuntimeException('Invalid git identifier detected. Identifier must not start with a -, given: ' . $identifier);
        }

                    if (Preg::isMatch('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match) && $match[1][0] !== '-') {
        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));
                if ($branch && Preg::isMatch('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match) && $match[1][0] !== '-') {
                if ($branch && Preg::isMatch('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match) && $match[1][0] !== '-') {
<?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);
    }
}
use Composer\Test\Mock\ProcessExecutorMock;

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