Fix Mercurial command injection vulnerability

Summary: See <http://chargen.matasano.com/chargen/2015/3/17/this-new-vulnerability-mercurial-command-injection-cve-2014-9462.html>.

Test Plan: Crafted bad remote URL; got error instead of code execution.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12112

src/__phutil_library_map__.php

@@ -2392,6 +2392,7 @@ phutil_register_library_map(array(
     'PhabricatorRepositoryURINormalizerTestCase' => 'applications/repository/data/__tests__/PhabricatorRepositoryURINormalizerTestCase.php',
     'PhabricatorRepositoryURITestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php',
     'PhabricatorRepositoryVCSPassword' => 'applications/repository/storage/PhabricatorRepositoryVCSPassword.php',
+    'PhabricatorRepositoryVersion' => 'applications/repository/constants/PhabricatorRepositoryVersion.php',
     'PhabricatorRobotsController' => 'applications/system/controller/PhabricatorRobotsController.php',
     'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php',
     'PhabricatorSMS' => 'infrastructure/sms/storage/PhabricatorSMS.php',
@@ -5757,6 +5758,7 @@ phutil_register_library_map(array(
     'PhabricatorRepositoryURINormalizerTestCase' => 'PhabricatorTestCase',
     'PhabricatorRepositoryURITestCase' => 'PhabricatorTestCase',
     'PhabricatorRepositoryVCSPassword' => 'PhabricatorRepositoryDAO',
+    'PhabricatorRepositoryVersion' => 'Phobject',
     'PhabricatorRobotsController' => 'PhabricatorController',
     'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine',
     'PhabricatorSMS' => 'PhabricatorSMSDAO',

src/applications/config/check/PhabricatorBinariesSetupCheck.php

@@ -121,18 +121,7 @@ final class PhabricatorBinariesSetupCheck extends PhabricatorSetupCheck {
             '2.2' => pht('This version of Mercurial has a significant memory '.
                          'leak, fixed in 2.2.1. Pushing fails with this '.
                          'version as well; see T3046#54922.'),);
-          list($err, $stdout, $stderr) = exec_manual('hg --version --quiet');
+          $version = PhabricatorRepositoryVersion::getMercurialVersion();
-
-          // NOTE: At least on OSX, recent versions of Mercurial report this
-          // string in this format:
-          //
-          //   Mercurial Distributed SCM (version 3.1.1+20140916)
-
-          $matches = null;
-          $pattern = '/^Mercurial Distributed SCM \(version ([\d.]+)/m';
-          if (preg_match($pattern, $stdout, $matches)) {
-            $version = $matches[1];
-          }
           break;
       }
 

src/applications/repository/constants/PhabricatorRepositoryVersion.php

@@ -0,0 +1,22 @@
+<?php
+
+final class PhabricatorRepositoryVersion extends Phobject {
+
+  public static function getMercurialVersion() {
+    list($err, $stdout, $stderr) = exec_manual('hg --version --quiet');
+
+    // NOTE: At least on OSX, recent versions of Mercurial report this
+    // string in this format:
+    //
+    //   Mercurial Distributed SCM (version 3.1.1+20140916)
+
+    $matches = null;
+    $pattern = '/^Mercurial Distributed SCM \(version ([\d.]+)/m';
+    if (preg_match($pattern, $stdout, $matches)) {
+      return $matches[1];
+    }
+
+    return null;
+  }
+
+}

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

@@ -367,9 +367,37 @@ final class PhabricatorRepositoryPullEngine
         'init -- %s',
         $path);
     } else {
+      $remote = $repository->getRemoteURIEnvelope();
+
+      // NOTE: Mercurial prior to 3.2.4 has an severe command injection
+      // vulnerability. See: <http://bit.ly/19B58E9>
+
+      // On vulnerable versions of Mercurial, we refuse to clone remotes which
+      // contain characters which may be interpreted by the shell.
+      $hg_version = PhabricatorRepositoryVersion::getMercurialVersion();
+      $is_vulnerable = version_compare($hg_version, '3.2.4', '<');
+      if ($is_vulnerable) {
+        $cleartext = $remote->openEnvelope();
+        // The use of "%R" here is an attempt to limit collateral damage
+        // for normal URIs because it isn't clear how long this vulnerability
+        // has been around for.
+
+        $escaped = csprintf('%R', $cleartext);
+        if ((string)$escaped !== (string)$cleartext) {
+          throw new Exception(
+            pht(
+              'You have an old version of Mercurial (%s) which has a severe '.
+              'command injection security vulnerability. The remote URI for '.
+              'this repository (%s) is potentially unsafe. Upgrade Mercurial '.
+              'to at least 3.2.4 to clone it.',
+              $hg_version,
+              $repository->getMonogram()));
+        }
+      }
+
       $repository->execxRemoteCommand(
         'clone --noupdate -- %P %s',
-        $repository->getRemoteURIEnvelope(),
+        $remote,
         $path);
     }
   }