diff --git a/src/markup/__tests__/PhutilMarkupTestCase.php b/src/markup/__tests__/PhutilMarkupTestCase.php --- a/src/markup/__tests__/PhutilMarkupTestCase.php +++ b/src/markup/__tests__/PhutilMarkupTestCase.php @@ -62,66 +62,6 @@ (string)phutil_tag('br', array('y' => null))); } - public function testDefaultRelNoreferrer() { - $map = array( - // These should not have rel="nofollow" inserted implicitly. - '/' => false, - '/path/to/local.html' => false, - '#example' => false, - '' => false, - - // These should get the implicit insertion. - 'http://www.example.org/' => true, - ' http://www.example.org/' => true, - 'ftp://filez.com' => true, - 'mailto:santa@northpole.com' => true, - 'tel:18005555555' => true, - - // These are protocol-relative hrefs. Browers will treat a URI with - // a leading slash followed by any positive number of slashes and - // backslashes as a protocol-relative link. - '//evil.com/' => true, - '/\\evil.com/' => true, - '///evil.com/' => true, - '//\\evil.com/' => true, - '/\\/evil.com/' => true, - '/\\\\/evil.com' => true, - ); - - foreach ($map as $input => $expect) { - $tag = phutil_tag( - 'a', - array( - 'href' => $input, - ), - 'link'); - $tag = (string)$tag; - $this->assertEqual($expect, (bool)preg_match('/noreferrer/', $tag)); - } - - // With an explicit `rel` present, we should not override it. - $tag = phutil_tag( - 'a', - array( - 'href' => 'http://www.example.org/', - 'rel' => 'nofollow', - ), - 'link'); - - $this->assertFalse((bool)preg_match('/noreferrer/', (string)$tag)); - - // For tags other than `a`, we should not insert `rel`. - $tag = phutil_tag( - 'link', - array( - 'href' => 'http://www.example.org/', - ), - 'link'); - - $this->assertFalse((bool)preg_match('/noreferrer/', (string)$tag)); - } - - public function testTagJavascriptProtocolRejection() { $hrefs = array( 'javascript:alert(1)' => true, diff --git a/src/markup/engine/__tests__/remarkup/link-noreferrer.txt b/src/markup/engine/__tests__/remarkup/link-noreferrer.txt new file mode 100644 --- /dev/null +++ b/src/markup/engine/__tests__/remarkup/link-noreferrer.txt @@ -0,0 +1,16 @@ +[[ /\evil.com ]] + +[[ / +/evil.com ]] + +~~~~~~~~~~ +

/\evil.com

+ +

/ +/evil.com

+~~~~~~~~~~ +/\evil.com + +/ +/evil.com diff --git a/src/markup/engine/remarkup/markuprule/PhutilRemarkupDocumentLinkRule.php b/src/markup/engine/remarkup/markuprule/PhutilRemarkupDocumentLinkRule.php --- a/src/markup/engine/remarkup/markuprule/PhutilRemarkupDocumentLinkRule.php +++ b/src/markup/engine/remarkup/markuprule/PhutilRemarkupDocumentLinkRule.php @@ -91,9 +91,10 @@ return phutil_tag( 'a', array( - 'href' => $link, - 'class' => 'remarkup-link', - 'target' => $target, + 'href' => $link, + 'class' => 'remarkup-link', + 'target' => $target, + 'rel' => 'noreferrer', ), $name); } diff --git a/src/markup/engine/remarkup/markuprule/PhutilRemarkupHyperlinkRule.php b/src/markup/engine/remarkup/markuprule/PhutilRemarkupHyperlinkRule.php --- a/src/markup/engine/remarkup/markuprule/PhutilRemarkupHyperlinkRule.php +++ b/src/markup/engine/remarkup/markuprule/PhutilRemarkupHyperlinkRule.php @@ -78,9 +78,10 @@ return phutil_tag( 'a', array( - 'href' => $link, - 'class' => 'remarkup-link', - 'target' => $target, + 'href' => $link, + 'class' => 'remarkup-link', + 'target' => $target, + 'rel' => 'noreferrer', ), $link); } diff --git a/src/markup/render.php b/src/markup/render.php --- a/src/markup/render.php +++ b/src/markup/render.php @@ -22,40 +22,13 @@ * @return PhutilSafeHTML Tag object. */ function phutil_tag($tag, array $attributes = array(), $content = null) { - // If the `href` attribute is present: - // - make sure it is not a "javascript:" URI. We never permit these. - // - if the tag is an `` and the link is to some foreign resource, - // add `rel="nofollow"` by default. + // If the `href` attribute is present, make sure it is not a "javascript:" + // URI. We never permit these. if (!empty($attributes['href'])) { - // This might be a URI object, so cast it to a string. $href = (string)$attributes['href']; if (isset($href[0])) { - $is_anchor_href = ($href[0] == '#'); - - // Is this a link to a resource on the same domain? The second part of - // this excludes "//evil.com/" protocol-relative hrefs. The third part - // of this excludes "/\evil.com/" protocol-relative fantasy hrefs which - // are completely made up but which browsers all respect. Broadly, - // browsers will dutifuly treat "/" followed by ANY sequence of "/" and - // "\" as though it were "//". - $is_domain_href = - ($href[0] == '/') && - (!isset($href[1]) || ($href[1] != '/' && $href[1] != '\\')); - - // If the `rel` attribute is not specified, fill in `rel="noreferrer"`. - // Effectively, this serves to make the default behavior for offsite - // links "do not send a referrer", which is broadly desirable. Specifying - // some non-null `rel` will skip this. - if (!isset($attributes['rel'])) { - if (!$is_anchor_href && !$is_domain_href) { - if ($tag == 'a') { - $attributes['rel'] = 'noreferrer'; - } - } - } - // Block 'javascript:' hrefs at the tag level: no well-designed // application should ever use them, and they are a potent attack vector. @@ -63,7 +36,7 @@ // doing a cheap version of this test first to avoid calling preg_match() // on URIs which begin with '/' or `#`. These cover essentially all URIs // in Phabricator. - if (!$is_anchor_href && !$is_domain_href) { + if (($href[0] !== '/') && ($href[0] !== '#')) { // Chrome 33 and IE 11 both interpret "javascript\n:" as a Javascript // URI, and all browsers interpret " javascript:" as a Javascript URI, // so be aggressive about looking for "javascript:" in the initial