Refactor HTML.Noopener to HTML.TargetNoopener so that it behaves like HTML.TargetNoreferrer and is active by default if a target is set

NEWS

@@ -19,7 +19,11 @@ NEWS ( CHANGELOG and HISTORY )                                     HTMLPurifier
 - Deleted some asserts to avoid linters from choking (#97)
 - Rework Serializer cache behavior to avoid chmod'ing if possible (#32)
 - Embedded semicolons in strings in CSS are now handled correctly!
-! Added %HTML.Noopener to add rel="noopener" to external links.
+# By default, when a link has a target attribute associated
+  with it, we now also add rel="noopener" in order to
+  prevent the new window from being able to overwrite
+  the original frame.  To disable this protection,
+  set %HTML.TargetNoopener to FALSE.
 
 4.8.0, released 2016-07-16
 # By default, when a link has a target attribute associated

configdoc/usage.xml

@@ -222,17 +222,17 @@
    <line>268</line>
   </file>
  </directive>
- <directive id="HTML.Noopener">
+ <directive id="HTML.TargetBlank">
   <file name="HTMLPurifier/HTMLModuleManager.php">
    <line>271</line>
   </file>
  </directive>
- <directive id="HTML.TargetBlank">
+ <directive id="HTML.TargetNoreferrer">
   <file name="HTMLPurifier/HTMLModuleManager.php">
-   <line>274</line>
+   <line>276</line>
   </file>
  </directive>
- <directive id="HTML.TargetNoreferrer">
+ <directive id="HTML.TargetNoopener">
   <file name="HTMLPurifier/HTMLModuleManager.php">
    <line>279</line>
   </file>

library/HTMLPurifier.includes.php

@@ -132,13 +132,13 @@ require 'HTMLPurifier/AttrTransform/Length.php';
 require 'HTMLPurifier/AttrTransform/Name.php';
 require 'HTMLPurifier/AttrTransform/NameSync.php';
 require 'HTMLPurifier/AttrTransform/Nofollow.php';
-require 'HTMLPurifier/AttrTransform/Noopener.php';
 require 'HTMLPurifier/AttrTransform/SafeEmbed.php';
 require 'HTMLPurifier/AttrTransform/SafeObject.php';
 require 'HTMLPurifier/AttrTransform/SafeParam.php';
 require 'HTMLPurifier/AttrTransform/ScriptRequired.php';
 require 'HTMLPurifier/AttrTransform/TargetBlank.php';
 require 'HTMLPurifier/AttrTransform/TargetNoreferrer.php';
+require 'HTMLPurifier/AttrTransform/TargetNoopener.php';
 require 'HTMLPurifier/AttrTransform/Textarea.php';
 require 'HTMLPurifier/ChildDef/Chameleon.php';
 require 'HTMLPurifier/ChildDef/Custom.php';
@@ -164,7 +164,6 @@ require 'HTMLPurifier/HTMLModule/Legacy.php';
 require 'HTMLPurifier/HTMLModule/List.php';
 require 'HTMLPurifier/HTMLModule/Name.php';
 require 'HTMLPurifier/HTMLModule/Nofollow.php';
-require 'HTMLPurifier/HTMLModule/Noopener.php';
 require 'HTMLPurifier/HTMLModule/NonXMLCommonAttributes.php';
 require 'HTMLPurifier/HTMLModule/Object.php';
 require 'HTMLPurifier/HTMLModule/Presentation.php';
@@ -179,6 +178,7 @@ require 'HTMLPurifier/HTMLModule/Tables.php';
 require 'HTMLPurifier/HTMLModule/Target.php';
 require 'HTMLPurifier/HTMLModule/TargetBlank.php';
 require 'HTMLPurifier/HTMLModule/TargetNoreferrer.php';
+require 'HTMLPurifier/HTMLModule/TargetNoopener.php';
 require 'HTMLPurifier/HTMLModule/Text.php';
 require 'HTMLPurifier/HTMLModule/Tidy.php';
 require 'HTMLPurifier/HTMLModule/XMLCommonAttributes.php';

library/HTMLPurifier.safe-includes.php

@@ -126,13 +126,13 @@ require_once $__dir . '/HTMLPurifier/AttrTransform/Length.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/Name.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/NameSync.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/Nofollow.php';
-require_once $__dir . '/HTMLPurifier/AttrTransform/Noopener.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/SafeEmbed.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/SafeObject.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/SafeParam.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/ScriptRequired.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/TargetBlank.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/TargetNoreferrer.php';
+require_once $__dir . '/HTMLPurifier/AttrTransform/TargetNoopener.php';
 require_once $__dir . '/HTMLPurifier/AttrTransform/Textarea.php';
 require_once $__dir . '/HTMLPurifier/ChildDef/Chameleon.php';
 require_once $__dir . '/HTMLPurifier/ChildDef/Custom.php';
@@ -158,7 +158,6 @@ require_once $__dir . '/HTMLPurifier/HTMLModule/Legacy.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/List.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/Name.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/Nofollow.php';
-require_once $__dir . '/HTMLPurifier/HTMLModule/Noopener.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/NonXMLCommonAttributes.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/Object.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/Presentation.php';
@@ -173,6 +172,7 @@ require_once $__dir . '/HTMLPurifier/HTMLModule/Tables.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/Target.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/TargetBlank.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/TargetNoreferrer.php';
+require_once $__dir . '/HTMLPurifier/HTMLModule/TargetNoopener.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/Text.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/Tidy.php';
 require_once $__dir . '/HTMLPurifier/HTMLModule/XMLCommonAttributes.php';

library/HTMLPurifier/AttrTransform/Noopener.php

@@ -1,52 +0,0 @@
-<?php
-
-// must be called POST validation
-
-/**
- * Adds rel="noopener" to all outbound links.  This transform is
- * only attached if Attr.Noopener is TRUE.
- */
-class HTMLPurifier_AttrTransform_Noopener extends HTMLPurifier_AttrTransform
-{
-    /**
-     * @type HTMLPurifier_URIParser
-     */
-    private $parser;
-
-    public function __construct()
-    {
-        $this->parser = new HTMLPurifier_URIParser();
-    }
-
-    /**
-     * @param array $attr
-     * @param HTMLPurifier_Config $config
-     * @param HTMLPurifier_Context $context
-     * @return array
-     */
-    public function transform($attr, $config, $context)
-    {
-        if (!isset($attr['href'])) {
-            return $attr;
-        }
-
-        // XXX Kind of inefficient
-        $url = $this->parser->parse($attr['href']);
-        $scheme = $url->getSchemeObj($config, $context);
-
-        if ($scheme->browsable && !$url->isLocal($config, $context)) {
-            if (isset($attr['rel'])) {
-                $rels = explode(' ', $attr['rel']);
-                if (!in_array('noopener', $rels)) {
-                    $rels[] = 'noopener';
-                }
-                $attr['rel'] = implode(' ', $rels);
-            } else {
-                $attr['rel'] = 'noopener';
-            }
-        }
-        return $attr;
-    }
-}
-
-// vim: et sw=4 sts=4

library/HTMLPurifier/AttrTransform/TargetNoopener.php

@@ -0,0 +1,37 @@
+<?php
+
+// must be called POST validation
+
+/**
+ * Adds rel="noopener" to any links which target a different window
+ * than the current one.  This is used to prevent malicious websites
+ * from silently replacing the original window, which could be used
+ * to do phishing.
+ * This transform is controlled by %HTML.TargetNoopener.
+ */
+class HTMLPurifier_AttrTransform_TargetNoopener extends HTMLPurifier_AttrTransform
+{
+    /**
+     * @param array $attr
+     * @param HTMLPurifier_Config $config
+     * @param HTMLPurifier_Context $context
+     * @return array
+     */
+    public function transform($attr, $config, $context)
+    {
+        if (isset($attr['rel'])) {
+            $rels = explode(' ', $attr['rel']);
+        } else {
+            $rels = array();
+        }
+        if (isset($attr['target']) && !in_array('noopener', $rels)) {
+            $rels[] = 'noopener';
+        }
+        if (!empty($rels) || isset($attr['rel'])) {
+            $attr['rel'] = implode(' ', $rels);
+        }
+
+        return $attr;
+    }
+}
+

library/HTMLPurifier/ConfigSchema/schema/HTML.Noopener.txt

@@ -1,7 +0,0 @@
-HTML.Noopener
-TYPE: bool
-VERSION: 4.9.0
-DEFAULT: FALSE
---DESCRIPTION--
-If enabled, noopener rel attributes are added to all outgoing links.
---# vim: et sw=4 sts=4

library/HTMLPurifier/ConfigSchema/schema/HTML.TargetNoopener.txt

@@ -0,0 +1,10 @@
+--# vim: et sw=4 sts=4
+HTML.TargetNoopener
+TYPE: bool
+VERSION: 4.8.0
+DEFAULT: TRUE
+--DESCRIPTION--
+If enabled, noopener rel attributes are added to links which have
+a target attribute associated with them.  This prevents malicious
+destinations from overwriting the original window.
+--# vim: et sw=4 sts=4

library/HTMLPurifier/HTMLModule/Noopener.php

@@ -1,25 +0,0 @@
-<?php
-
-/**
- * Module adds the noopener attribute transformation to a tags.  It
- * is enabled by HTML.Noopener
- */
-class HTMLPurifier_HTMLModule_Noopener extends HTMLPurifier_HTMLModule
-{
-
-    /**
-     * @type string
-     */
-    public $name = 'Noopener';
-
-    /**
-     * @param HTMLPurifier_Config $config
-     */
-    public function setup($config)
-    {
-        $a = $this->addBlankElement('a');
-        $a->attr_transform_post[] = new HTMLPurifier_AttrTransform_Noopener();
-    }
-}
-
-// vim: et sw=4 sts=4

library/HTMLPurifier/HTMLModule/TargetNoopener.php

@@ -0,0 +1,21 @@
+<?php
+
+/**
+ * Module adds the target-based noopener attribute transformation to a tags.  It
+ * is enabled by HTML.TargetNoopener
+ */
+class HTMLPurifier_HTMLModule_TargetNoopener extends HTMLPurifier_HTMLModule
+{
+    /**
+     * @type string
+     */
+    public $name = 'TargetNoopener';
+
+    /**
+     * @param HTMLPurifier_Config $config
+     */
+    public function setup($config) {
+        $a = $this->addBlankElement('a');
+        $a->attr_transform_post[] = new HTMLPurifier_AttrTransform_TargetNoopener();
+    }
+}

library/HTMLPurifier/HTMLModuleManager.php

@@ -268,17 +268,17 @@ class HTMLPurifier_HTMLModuleManager
         if ($config->get('HTML.Nofollow')) {
             $modules[] = 'Nofollow';
         }
-        if ($config->get('HTML.Noopener')) {
-            $modules[] = 'Noopener';
-        }
         if ($config->get('HTML.TargetBlank')) {
             $modules[] = 'TargetBlank';
         }
-        // NB: HTML.TargetNoreferrer must be AFTER HTML.TargetBlank
+        // NB: HTML.TargetNoreferrer and HTML.TargetNoopener must be AFTER HTML.TargetBlank
         // so that its post-attr-transform gets run afterwards.
         if ($config->get('HTML.TargetNoreferrer')) {
             $modules[] = 'TargetNoreferrer';
         }
+        if ($config->get('HTML.TargetNoopener')) {
+            $modules[] = 'TargetNoopener';
+        }
 
         // merge in custom modules
         $modules = array_merge($modules, $this->userModules);

tests/HTMLPurifier/HTMLModule/NoopenerTest.php

@@ -1,30 +0,0 @@
-<?php
-
-class HTMLPurifier_HTMLModule_NoopenerTest extends HTMLPurifier_HTMLModuleHarness
-{
-
-    public function setUp()
-    {
-        parent::setUp();
-        $this->config->set('HTML.Noopener', true);
-        $this->config->set('Attr.AllowedRel', array("noopener", "blah"));
-    }
-
-    public function testNoopener()
-    {
-        $this->assertResult(
-            '<a href="http://google.com">x</a><a href="http://google.com" rel="blah">a</a><a href="/local">b</a><a href="mailto:foo@example.com">c</a>',
-            '<a href="http://google.com" rel="noopener">x</a><a href="http://google.com" rel="blah noopener">a</a><a href="/local">b</a><a href="mailto:foo@example.com">c</a>'
-        );
-    }
-
-    public function testNoopenerDupe()
-    {
-        $this->assertResult(
-            '<a href="http://google.com" rel="noopener">x</a><a href="http://google.com" rel="blah noopener">a</a><a href="/local">b</a><a href="mailto:foo@example.com">c</a>'
-        );
-    }
-
-}
-
-// vim: et sw=4 sts=4

tests/HTMLPurifier/HTMLModule/TargetBlankTest.php

@@ -13,7 +13,14 @@ class HTMLPurifier_HTMLModule_TargetBlankTest extends HTMLPurifier_HTMLModuleHar
     {
         $this->assertResult(
             '<a href="http://google.com">a</a><a href="/local">b</a><a href="mailto:foo@example.com">c</a>',
-            '<a href="http://google.com" target="_blank" rel="noreferrer">a</a><a href="/local">b</a><a href="mailto:foo@example.com">c</a>'
+            '<a href="http://google.com" target="_blank" rel="noreferrer noopener">a</a><a href="/local">b</a><a href="mailto:foo@example.com">c</a>'
+        );
+    }
+
+    public function testTargetBlankNoDupe() {
+        $this->assertResult(
+            '<a href="http://google.com" target="_blank">a</a>',
+            '<a href="http://google.com" target="_blank" rel="noreferrer noopener">a</a>'
         );
     }
 

tests/HTMLPurifier/HTMLModule/TargetNoopenerTest.php

@@ -0,0 +1,51 @@
+<?php
+
+class HTMLPurifier_HTMLModule_TargetNoopenerTest extends HTMLPurifier_HTMLModuleHarness
+{
+
+    public function setUp()
+    {
+        parent::setUp();
+        $this->config->set('HTML.TargetNoreferrer', false);
+        $this->config->set('HTML.TargetNoopener', true);
+        $this->config->set('Attr.AllowedFrameTargets', '_blank');
+    }
+
+    public function testNoreferrer()
+    {
+        $this->assertResult(
+            '<a href="http://google.com" target="_blank">x</a>',
+            '<a href="http://google.com" target="_blank" rel="noopener">x</a>'
+        );
+    }
+
+    public function testNoreferrerNoDupe()
+    {
+        $this->config->set('Attr.AllowedRel', 'noopener');
+        $this->assertResult(
+            '<a href="http://google.com" target="_blank" rel="noopener">x</a>',
+            '<a href="http://google.com" target="_blank" rel="noopener">x</a>'
+        );
+    }
+
+    public function testTargetBlankNoreferrer()
+    {
+        $this->config->set('HTML.TargetBlank', true);
+        $this->assertResult(
+            '<a href="http://google.com">x</a>',
+            '<a href="http://google.com" target="_blank" rel="noopener">x</a>'
+        );
+    }
+
+    public function testNoTarget()
+    {
+        $this->assertResult(
+            '<a href="http://google.com">x</a>',
+            '<a href="http://google.com">x</a>'
+        );
+    }
+
+
+}
+
+// vim: et sw=4 sts=4

tests/HTMLPurifier/HTMLModule/TargetNoreferrerTest.php

@@ -7,6 +7,7 @@ class HTMLPurifier_HTMLModule_TargetNoreferrerTest extends HTMLPurifier_HTMLModu
     {
         parent::setUp();
         $this->config->set('HTML.TargetNoreferrer', true);
+        $this->config->set('HTML.TargetNoopener', false);
         $this->config->set('Attr.AllowedFrameTargets', '_blank');
     }
 
@@ -36,6 +37,14 @@ class HTMLPurifier_HTMLModule_TargetNoreferrerTest extends HTMLPurifier_HTMLModu
         );
     }
 
+    public function testNoTarget()
+    {
+        $this->assertResult(
+            '<a href="http://google.com">x</a>',
+            '<a href="http://google.com">x</a>'
+        );
+    }
+
 
 }
 

tests/HTMLPurifier/Strategy/ValidateAttributesTest.php

@@ -189,7 +189,7 @@ class HTMLPurifier_Strategy_ValidateAttributesTest extends
     {
         $this->config->set('Attr.AllowedFrameTargets', '_top');
         $this->config->set('HTML.Doctype', 'XHTML 1.0 Transitional');
-        $this->assertResult('<a href="foo" target="_top" rel="noreferrer" />');
+        $this->assertResult('<a href="foo" target="_top" rel="noreferrer noopener" />');
     }
 
     public function testRemoveTargetWhenNotSupported()