Page MenuHomePhabricator
Files F15466167

D19117.id45811.diff
No OneTemporary
Actions

D19117.id45811.diff
View Options

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 ]]
+
+~~~~~~~~~~
+<p><a href="/\evil.com" class="remarkup-link" target="_blank" rel="noreferrer">/\evil.com</a></p>
+
+<p><a href="/
+/evil.com" class="remarkup-link" target="_blank" rel="noreferrer">/
+/evil.com</a></p>
+~~~~~~~~~~
+/\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 `<a>` 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

File Metadata

Mime Type
text/plain
Expires
Fri, Apr 4, 2:23 AM (2 d, 10 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7394594
Default Alt Text
D19117.id45811.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility