diff --git a/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php b/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php index a16416ae1d..168b18caa5 100644 --- a/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php @@ -26,9 +26,7 @@ final class DiffusionGitCommandEngine $env['HOME'] = PhabricatorEnv::getEmptyCWD(); - if ($this->isAnySSHProtocol()) { - $env['GIT_SSH'] = $this->getSSHWrapper(); - } + $env['GIT_SSH'] = $this->getSSHWrapper(); if ($this->isAnyHTTPProtocol()) { $uri = $this->getURI(); diff --git a/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php b/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php index 49c618b085..9499b8f5f1 100644 --- a/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php @@ -11,12 +11,14 @@ final class DiffusionMercurialCommandEngine protected function newFormattedCommand($pattern, array $argv) { $args = array(); - if ($this->isAnySSHProtocol()) { - $pattern = "hg --config ui.ssh=%s {$pattern}"; - $args[] = $this->getSSHWrapper(); - } else { - $pattern = "hg {$pattern}"; - } + // NOTE: Here, and in Git and Subversion, we override the SSH command even + // if the repository does not use an SSH remote, since our SSH wrapper + // defuses an attack against older versions of Mercurial, Git and + // Subversion (see T12961) and it's possible to execute this attack + // in indirect ways, like by using an SSH subrepo inside an HTTP repo. + + $pattern = "hg --config ui.ssh=%s {$pattern}"; + $args[] = $this->getSSHWrapper(); return array($pattern, array_merge($args, $argv)); } diff --git a/src/applications/diffusion/protocol/DiffusionSubversionCommandEngine.php b/src/applications/diffusion/protocol/DiffusionSubversionCommandEngine.php index d8e9e6ff17..75b8785a6e 100644 --- a/src/applications/diffusion/protocol/DiffusionSubversionCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionSubversionCommandEngine.php @@ -44,9 +44,7 @@ final class DiffusionSubversionCommandEngine protected function newCustomEnvironment() { $env = array(); - if ($this->isAnySSHProtocol()) { - $env['SVN_SSH'] = $this->getSSHWrapper(); - } + $env['SVN_SSH'] = $this->getSSHWrapper(); return $env; } diff --git a/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php b/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php index 76d9ad9170..1df3ea3522 100644 --- a/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php +++ b/src/applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php @@ -26,7 +26,7 @@ final class DiffusionCommandEngineTestCase extends PhabricatorTestCase { )); $this->assertCommandEngineFormat( - 'hg xyz', + (string)csprintf('hg --config ui.ssh=%s xyz', $ssh_wrapper), array( 'LANG' => 'en_US.UTF-8', 'HGPLAIN' => '1', @@ -102,7 +102,7 @@ final class DiffusionCommandEngineTestCase extends PhabricatorTestCase { )); $this->assertCommandEngineFormat( - 'hg xyz', + (string)csprintf('hg --config ui.ssh=%s xyz', $ssh_wrapper), array( 'LANG' => 'en_US.UTF-8', 'HGPLAIN' => '1',