diff --git a/conf/default.conf.php b/conf/default.conf.php index c092200026..2b9eaf0fd1 100644 --- a/conf/default.conf.php +++ b/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: diff --git a/src/applications/markup/engine/PhabricatorMarkupEngine.php b/src/applications/markup/engine/PhabricatorMarkupEngine.php index 4d895f8e2f..788774bd6b 100644 --- a/src/applications/markup/engine/PhabricatorMarkupEngine.php +++ b/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) { diff --git a/src/infrastructure/markup/remarkup/markuprule/phriction/PhabricatorRemarkupRulePhriction.php b/src/infrastructure/markup/remarkup/markuprule/phriction/PhabricatorRemarkupRulePhriction.php index 7bcc188a8f..14a67a8f7f 100644 --- a/src/infrastructure/markup/remarkup/markuprule/phriction/PhabricatorRemarkupRulePhriction.php +++ b/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 diff --git a/src/infrastructure/markup/remarkup/markuprule/phriction/__init__.php b/src/infrastructure/markup/remarkup/markuprule/phriction/__init__.php index 1c70aaadfc..f93c3ed5c5 100644 --- a/src/infrastructure/markup/remarkup/markuprule/phriction/__init__.php +++ b/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');