From 05a4c55c52be313c750b1552b89ad33bcf64a0dd Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 17 Feb 2018 17:37:38 -0800 Subject: [PATCH] Explicitly add rel="noreferrer" to all external links Summary: See D19117. Instead of automatically figuring this out inside `phutil_tag()`, explicitly add rel="noreferrer" at the application level to all external links. Test Plan: - Grepped for `_blank`, `isValidRemoteURIForLink`, checked all callsites for user-controlled data. - Created a link menu item, verified noreferrer in markup. - Created a link custom field, verified no referrer in markup. - Verified noreferrer for `{nav href=...}`. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Differential Revision: https://secure.phabricator.com/D19118 --- .../auth/view/PhabricatorAuthAccountView.php | 1 + .../PhabricatorCalendarICSURIImportEngine.php | 1 + .../markup/PhabricatorImageRemarkupRule.php | 8 +----- .../artifact/HarbormasterURIArtifact.php | 1 + .../nuance/item/NuanceGitHubEventItemType.php | 2 ++ .../PhabricatorPhurlLinkRemarkupRule.php | 1 + .../PhabricatorLinkProfileMenuItem.php | 3 ++- .../PhabricatorStandardCustomFieldLink.php | 6 ++++- src/view/layout/PhabricatorActionView.php | 3 +++ src/view/phui/PHUIListItemView.php | 13 +++++++++- src/view/phui/PHUITagView.php | 25 +++++++++++-------- 11 files changed, 44 insertions(+), 20 deletions(-) diff --git a/src/applications/auth/view/PhabricatorAuthAccountView.php b/src/applications/auth/view/PhabricatorAuthAccountView.php index 8eb73144aa..3f04281c8f 100644 --- a/src/applications/auth/view/PhabricatorAuthAccountView.php +++ b/src/applications/auth/view/PhabricatorAuthAccountView.php @@ -77,6 +77,7 @@ final class PhabricatorAuthAccountView extends AphrontView { array( 'href' => $account_uri, 'target' => '_blank', + 'rel' => 'noreferrer', ), $account_uri); } diff --git a/src/applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php index 6174603c84..bd52ec5bc2 100644 --- a/src/applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php @@ -45,6 +45,7 @@ final class PhabricatorCalendarICSURIImportEngine array( 'href' => $uri, 'target' => '_blank', + 'rel' => 'noreferrer', ), $uri); } diff --git a/src/applications/files/markup/PhabricatorImageRemarkupRule.php b/src/applications/files/markup/PhabricatorImageRemarkupRule.php index 9e91bdc096..36089dc57e 100644 --- a/src/applications/files/markup/PhabricatorImageRemarkupRule.php +++ b/src/applications/files/markup/PhabricatorImageRemarkupRule.php @@ -20,7 +20,6 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { $defaults = array( 'uri' => null, 'alt' => null, - 'href' => null, 'width' => null, 'height' => null, ); @@ -45,10 +44,6 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { $args += $defaults; - if ($args['href'] && !PhabricatorEnv::isValidURIForLink($args['href'])) { - $args['href'] = null; - } - if ($args['uri']) { $src_uri = id(new PhutilURI('/file/imageproxy/')) ->setQueryParam('uri', (string)$args['uri']); @@ -57,10 +52,9 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { array( 'src' => $src_uri, 'alt' => $args['alt'], - 'href' => $args['href'], 'width' => $args['width'], 'height' => $args['height'], - )); + )); return $this->getEngine()->storeText($img); } else { return $matches[0]; diff --git a/src/applications/harbormaster/artifact/HarbormasterURIArtifact.php b/src/applications/harbormaster/artifact/HarbormasterURIArtifact.php index 345621f0f5..93f7564033 100644 --- a/src/applications/harbormaster/artifact/HarbormasterURIArtifact.php +++ b/src/applications/harbormaster/artifact/HarbormasterURIArtifact.php @@ -81,6 +81,7 @@ final class HarbormasterURIArtifact extends HarbormasterArtifact { array( 'href' => $uri, 'target' => '_blank', + 'rel' => 'noreferrer', ), $name); } diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php index b8bfb3ccab..1f6249f222 100644 --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -309,6 +309,8 @@ final class NuanceGitHubEventItemType 'a', array( 'href' => $event_uri, + 'target' => '_blank', + 'rel' => 'noreferrer', ), $event_uri); } diff --git a/src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php b/src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php index 4ef59b300c..c4ffc366da 100644 --- a/src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php +++ b/src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php @@ -64,6 +64,7 @@ final class PhabricatorPhurlLinkRemarkupRule extends PhutilRemarkupRule { array( 'href' => $uri, 'target' => '_blank', + 'rel' => 'noreferrer', ), $name); } diff --git a/src/applications/search/menuitem/PhabricatorLinkProfileMenuItem.php b/src/applications/search/menuitem/PhabricatorLinkProfileMenuItem.php index f790816c8b..0b6a2f330e 100644 --- a/src/applications/search/menuitem/PhabricatorLinkProfileMenuItem.php +++ b/src/applications/search/menuitem/PhabricatorLinkProfileMenuItem.php @@ -99,7 +99,8 @@ final class PhabricatorLinkProfileMenuItem ->setHref($href) ->setName($name) ->setIcon($icon_class) - ->setTooltip($tooltip); + ->setTooltip($tooltip) + ->setRel('noreferrer'); return array( $item, diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php index c2b9d6543c..146f34ab07 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php @@ -31,7 +31,11 @@ final class PhabricatorStandardCustomFieldLink return phutil_tag( 'a', - array('href' => $value, 'target' => '_blank'), + array( + 'href' => $value, + 'target' => '_blank', + 'rel' => 'noreferrer', + ), $value); } diff --git a/src/view/layout/PhabricatorActionView.php b/src/view/layout/PhabricatorActionView.php index f6de8eca5b..a1d8fe2664 100644 --- a/src/view/layout/PhabricatorActionView.php +++ b/src/view/layout/PhabricatorActionView.php @@ -255,8 +255,10 @@ final class PhabricatorActionView extends AphrontView { } else { if ($this->getOpenInNewWindow()) { $target = '_blank'; + $rel = 'noreferrer'; } else { $target = null; + $rel = null; } if ($this->submenu) { @@ -277,6 +279,7 @@ final class PhabricatorActionView extends AphrontView { 'href' => $this->getHref(), 'class' => 'phabricator-action-view-item', 'target' => $target, + 'rel' => $rel, 'sigil' => $sigils, 'meta' => $this->metadata, ), diff --git a/src/view/phui/PHUIListItemView.php b/src/view/phui/PHUIListItemView.php index e8a1940737..8e53024826 100644 --- a/src/view/phui/PHUIListItemView.php +++ b/src/view/phui/PHUIListItemView.php @@ -34,6 +34,7 @@ final class PHUIListItemView extends AphrontTagView { private $actionIcon; private $actionIconHref; private $count; + private $rel; public function setOpenInNewWindow($open_in_new_window) { $this->openInNewWindow = $open_in_new_window; @@ -44,7 +45,16 @@ final class PHUIListItemView extends AphrontTagView { return $this->openInNewWindow; } - public function setHideInApplicationMenu($hide) { + public function setRel($rel) { + $this->rel = $rel; + return $this; + } + + public function getRel() { + return $this->rel; + } + + public function setHideInApplicationMenu($hide) { $this->hideInApplicationMenu = $hide; return $this; } @@ -363,6 +373,7 @@ final class PHUIListItemView extends AphrontTagView { 'meta' => $meta, 'sigil' => $sigil, 'target' => $this->getOpenInNewWindow() ? '_blank' : null, + 'rel' => $this->rel, ), array( $aural, diff --git a/src/view/phui/PHUITagView.php b/src/view/phui/PHUITagView.php index 292246c4a7..482b3f4400 100644 --- a/src/view/phui/PHUITagView.php +++ b/src/view/phui/PHUITagView.php @@ -154,25 +154,30 @@ final class PHUITagView extends AphrontTagView { $classes[] = 'phui-tag-'.$this->border; } + $attributes = array( + 'href' => $this->href, + 'class' => $classes, + ); + + if ($this->external) { + $attributes += array( + 'target' => '_blank', + 'rel' => 'noreferrer', + ); + } + if ($this->phid) { Javelin::initBehavior('phui-hovercards'); - $attributes = array( - 'href' => $this->href, + $attributes += array( 'sigil' => 'hovercard', - 'meta' => array( + 'meta' => array( 'hoverPHID' => $this->phid, ), - 'target' => $this->external ? '_blank' : null, - ); - } else { - $attributes = array( - 'href' => $this->href, - 'target' => $this->external ? '_blank' : null, ); } - return $attributes + array('class' => $classes); + return $attributes; } protected function getTagContent() { -- 2.51.2