Provide a markup protocol whitelist for Phabricator

Summary: See T548 and D996. Makes Phabricator configure the remarkup engine so
http:// and https:// get linked. Also make the "named link" syntax respect the
whitelist.

Test Plan:
  - Whitelisted URIs (they get linked).
  - Other URIs (not linked).
  - Whitelisted, named URIs (linked).
  - Other, named URIs (treated as phriction links).
  - Actual phriction links (work correctly).

Reviewers: jungejason, nh, tuomaspelkonen, aran, benmathews

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 997

conf/default.conf.php

@@ -402,6 +402,14 @@ return array(
   // You can enable traces for development to make it easier to debug problems.
   'phabricator.show-stack-traces' => false,
 
+  // When users write comments which have URIs, they'll be automaticaly linked
+  // if the protocol appears in this set. This whitelist is primarily to prevent
+  // security issues like javascript:// URIs.
+  'uri.allowed-protocols' => array(
+    'http'  => true,
+    'https' => true,
+  ),
+
   // Tokenizers are UI controls which let the user select other users, email
   // addresses, project names, etc., by typing the first few letters and having
   // the control autocomplete from a list. They can load their data in two ways:

src/applications/markup/engine/PhabricatorMarkupEngine.php

@@ -76,6 +76,8 @@ class PhabricatorMarkupEngine {
       'custom-inline' => array(),
       'custom-block'  => array(),
       'macros'        => true,
+      'uri.allowed-protocols' => PhabricatorEnv::getEnvConfig(
+        'uri.allowed-protocols'),
     );
   }
 
@@ -87,6 +89,9 @@ class PhabricatorMarkupEngine {
 
     $engine->setConfig('preserve-linebreaks', true);
     $engine->setConfig('pygments.enabled', $options['pygments']);
+    $engine->setConfig(
+      'uri.allowed-protocols',
+      $options['uri.allowed-protocols']);
 
     $rules = array();
     $rules[] = new PhutilRemarkupRuleEscapeRemarkup();
@@ -98,6 +103,7 @@ class PhabricatorMarkupEngine {
       $rules[] = new PhabricatorRemarkupRuleYoutube();
     }
 
+    $rules[] = new PhabricatorRemarkupRulePhriction();
     $rules[] = new PhutilRemarkupRuleHyperlink();
 
     $rules[] = new PhabricatorRemarkupRuleDifferentialHandle();
@@ -115,7 +121,6 @@ class PhabricatorMarkupEngine {
     }
 
     $rules[] = new PhabricatorRemarkupRuleMention();
-    $rules[] = new PhabricatorRemarkupRulePhriction();
 
     $custom_rule_classes = $options['custom-inline'];
     if ($custom_rule_classes) {

src/infrastructure/markup/remarkup/markuprule/phriction/PhabricatorRemarkupRulePhriction.php

@@ -37,6 +37,18 @@ class PhabricatorRemarkupRulePhriction
     // If whatever is being linked to begins with "/" or has "://", treat it
     // as a URI instead of a wiki page.
     $is_uri = preg_match('@(^/)|(://)@', $slug);
+
+    if ($is_uri) {
+      $protocols = $this->getEngine()->getConfig(
+        'uri.allowed-protocols',
+        array());
+      $protocol = id(new PhutilURI($slug))->getProtocol();
+      if (!idx($protocols, $protocol)) {
+        // Don't treat this as a URI if it's not an allowed protocol.
+        $is_uri = false;
+      }
+    }
+
     if ($is_uri) {
       $uri = $slug;
       // Leave the name unchanged, i.e. link the whole URI if there's no

src/infrastructure/markup/remarkup/markuprule/phriction/__init__.php

@@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/phriction/storage/document');
 
 phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'markup/engine/remarkup/markuprule/base');
+phutil_require_module('phutil', 'parser/uri');
 phutil_require_module('phutil', 'utils');