Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix escaping issues on Windows which could lead to command injection,…
  • Loading branch information
Seldaek committed Oct 5, 2021
1 parent 1a994e4 commit ca5e2f8
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 36 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
@@ -1,3 +1,7 @@
### [1.10.23] 2021-10-05

* Security: Fixed command injection vulnerability on Windows (GHSA-frqg-7g38-6gcf / CVE-2021-41116)

### [1.10.22] 2021-04-27

* Security: Fixed command injection vulnerability in HgDriver/HgDownloader and hardened other VCS drivers and downloaders (GHSA-h5h8-pc6h-jvvx / CVE-2021-29472)
Expand Down Expand Up @@ -938,6 +942,7 @@

* Initial release

[1.10.23]: https://github.com/composer/composer/compare/1.10.22...1.10.23
[1.10.22]: https://github.com/composer/composer/compare/1.10.21...1.10.22
[1.10.21]: https://github.com/composer/composer/compare/1.10.20...1.10.21
[1.10.20]: https://github.com/composer/composer/compare/1.10.19...1.10.20
Expand Down
2 changes: 1 addition & 1 deletion src/Composer/Command/HomeCommand.php
Expand Up @@ -129,7 +129,7 @@ private function openBrowser($url)

$process = new ProcessExecutor($this->getIO());
if (Platform::isWindows()) {
return $process->execute('start "web" explorer "' . $url . '"', $output);
return $process->execute('start "web" explorer ' . $url, $output);
}

$linux = $process->execute('which xdg-open', $output);
Expand Down
47 changes: 14 additions & 33 deletions src/Composer/Util/ProcessExecutor.php
Expand Up @@ -155,48 +155,29 @@ public static function escape($argument)
}

/**
* Copy of ProcessUtils::escapeArgument() that is deprecated in Symfony 3.3 and removed in Symfony 4.
* Copy of Symfony's Process::escapeArgument() which is private
*
* @param string $argument
*
* @return string
*/
private static function escapeArgument($argument)
{
//Fix for PHP bug #43784 escapeshellarg removes % from given string
//Fix for PHP bug #49446 escapeshellarg doesn't work on Windows
//@see https://bugs.php.net/bug.php?id=43784
//@see https://bugs.php.net/bug.php?id=49446
if ('\\' === DIRECTORY_SEPARATOR) {
if ((string) $argument === '') {
return escapeshellarg($argument);
}

$escapedArgument = '';
$quote = false;
foreach (preg_split('/(")/', $argument, -1, PREG_SPLIT_NO_EMPTY | PREG_SPLIT_DELIM_CAPTURE) as $part) {
if ('"' === $part) {
$escapedArgument .= '\\"';
} elseif (self::isSurroundedBy($part, '%')) {
// Avoid environment variable expansion
$escapedArgument .= '^%"'.substr($part, 1, -1).'"^%';
} else {
// escape trailing backslash
if ('\\' === substr($part, -1)) {
$part .= '\\';
}
$quote = true;
$escapedArgument .= $part;
}
}
if ($quote) {
$escapedArgument = '"'.$escapedArgument.'"';
}

return $escapedArgument;
if ('' === $argument || null === $argument) {
return '""';
}
if ('\\' !== \DIRECTORY_SEPARATOR) {
return "'".str_replace("'", "'\\''", $argument)."'";
}
if (false !== strpos($argument, "\0")) {
$argument = str_replace("\0", '?', $argument);
}
if (!preg_match('/[\/()%!^"<>&|\s]/', $argument)) {
return $argument;
}
$argument = preg_replace('/(\\\\+)$/', '$1$1', $argument);

return "'".str_replace("'", "'\\''", $argument)."'";
return '"'.str_replace(array('"', '^', '%', '!', "\n"), array('""', '"^^"', '"^%"', '"^!"', '!LF!'), $argument).'"';
}

private static function isSurroundedBy($arg, $char)
Expand Down
2 changes: 1 addition & 1 deletion tests/Composer/Test/Downloader/GitDownloaderTest.php
Expand Up @@ -524,7 +524,7 @@ public function testUpdateThrowsRuntimeExceptionIfGitCommandFails()

public function testUpdateDoesntThrowsRuntimeExceptionIfGitCommandFailsAtFirstButIsAbleToRecover()
{
$expectedFirstGitUpdateCommand = $this->winCompat("(git remote set-url composer -- '' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- ''");
$expectedFirstGitUpdateCommand = $this->winCompat("(git remote set-url composer -- \"\" && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- \"\"");
$expectedSecondGitUpdateCommand = $this->winCompat("(git remote set-url composer -- 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- 'https://github.com/composer/composer'");

$packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock();
Expand Down
2 changes: 1 addition & 1 deletion tests/Composer/Test/Util/SvnTest.php
Expand Up @@ -47,7 +47,7 @@ public function urlProvider()
return array(
array('http://till:test@svn.example.org/', $this->getCmd(" --username 'till' --password 'test' ")),
array('http://svn.apache.org/', ''),
array('svn://johndoe@example.org', $this->getCmd(" --username 'johndoe' --password '' ")),
array('svn://johndoe@example.org', $this->getCmd(" --username 'johndoe' --password \"\" ")),
);
}

Expand Down

0 comments on commit ca5e2f8

Please sign in to comment.