SONARHTML-220 Fix FP S5148: Only raise an issue if rel="opener" is ex…

…plicitly set (#337)

its/ruling/src/test/resources/expected/Web-HeaderCheck.json

@@ -1562,6 +1562,9 @@
 "project:Silverpeas-Core-master/war-core/src/test/webapp/util/javaScript/silverpeas-datechecker.test.html": [
 0
 ],
+"project:custom/S5148.html": [
+0
+],
 "project:custom/S6842.html": [
 0
 ],

its/ruling/src/test/resources/expected/Web-S1829.json

@@ -1372,6 +1372,9 @@
 81,
 81
 ],
+"project:custom/S5148.html": [
+1
+],
 "project:external_webkit-jb-mr1/LayoutTests/dom/html/level1/core/resources/COPYRIGHT.html": [
 18,
 19,

its/ruling/src/test/resources/expected/Web-S5148.json

@@ -1,65 +1,5 @@
 {
-"project:Silverpeas-Core-master/war-core/src/main/webapp/admin/jsp/TopBarSilverpeasV5.jsp": [
-181
-],
-"project:Silverpeas-Core-master/war-core/src/main/webapp/attachment/jsp/displayAttachedFiles.jsp": [
-245
-],
-"project:Silverpeas-Core-master/war-core/src/main/webapp/attachment/jsp/editAttachedFiles.jsp": [
-465,
-473
-],
-"project:Silverpeas-Core-master/war-core/src/main/webapp/jobManagerPeas/jsp/topBarManager.jsp": [
-77
-],
-"project:external_webkit-jb-mr1/LayoutTests/fast/encoding/hanarei-blog32-fc2-com.html": [
-20
-],
-"project:sonar-master/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/issues.html.erb": [
-143
-],
-"project:sonar-master/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/measures.html.erb": [
-65
-],
-"project:voten/resources/assets/js/components/GuestSidebar.vue": [
-109,
-116,
-132
-],
-"project:voten/resources/assets/js/components/MarkdownGuide.vue": [
-49
-],
-"project:voten/resources/assets/js/components/SearchModal.vue": [
-37
-],
-"project:voten/resources/assets/js/components/auth/LeftSidebar.vue": [
-134,
-177
-],
-"project:voten/resources/assets/js/components/auth/RightSidebar.vue": [
-55,
-79,
-88
-],
-"project:voten/resources/assets/js/components/pages/About.vue": [
-44
-],
-"project:voten/resources/views/backend/spams/comments.blade.php": [
-46
-],
-"project:voten/resources/views/backend/spams/submissions.blade.php": [
-46
-],
-"project:voten/resources/views/vendor/mail/html/button.blade.php": [
-10
-],
-"project:voten/resources/views/vendor/mail/html/promotion/button.blade.php": [
-7
-],
-"project:voten/resources/views/vendor/notifications/email.blade.php": [
-73,
-123,
-157,
-179
+"project:custom/S5148.html": [
+1
 ]
 }

sonar-html-plugin/src/main/java/org/sonar/plugins/html/checks/sonar/LinkWithTargetBlankCheck.java

@@ -18,8 +18,6 @@
 package org.sonar.plugins.html.checks.sonar;
 
 import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
 import java.util.Locale;
 import java.util.regex.Pattern;
 import org.sonar.check.Rule;
@@ -30,31 +28,35 @@
 public class LinkWithTargetBlankCheck extends AbstractPageCheck {
 
   private static final Pattern DYNAMIC_URL = Pattern.compile("[{}$()\\[\\]]");
-  private static final String NOOPENER = "NOOPENER";
 
   @Override
   public void startElement(TagNode node) {
-    if (isAnchorWithTargetBlank(node) && isInsecureUrl(node) && missingRelAttribute(node)) {
-      createViolation(node, "Make sure not using rel=\"noopener\" is safe here.");
+    if (isAnchor(node) && isInsecureUrl(node) && isVulnerable(node)) {
+      createViolation(node, "Make sure using target=\"_blank\" and rel=\"opener\" is safe here.");
     }
   }
 
-  private static boolean missingRelAttribute(TagNode node) {
-    return !relAttributeValues(node).contains(NOOPENER);
+  private static boolean isVulnerable(TagNode node) {
+    return hasBlankTarget(node) && hasRelOpener(node);
   }
 
-  private static List<String> relAttributeValues(TagNode node) {
+  private static boolean hasBlankTarget(TagNode node) {
+    String target = node.getPropertyValue("TARGET");
+    return "_blank".equalsIgnoreCase(target);
+  }
+
+  private static boolean hasRelOpener(TagNode node) {
     String rel = node.getPropertyValue("REL");
     if (rel == null) {
-      return Collections.emptyList();
+      return false;
     }
     return Arrays.stream(rel.split(" "))
       .map(s -> s.trim().toUpperCase(Locale.ROOT))
-      .toList();
+      .anyMatch("opener"::equalsIgnoreCase);
   }
 
-  private static boolean isAnchorWithTargetBlank(TagNode node) {
-    return node.equalsElementName("A") && "_BLANK".equalsIgnoreCase(node.getPropertyValue("TARGET"));
+  private static boolean isAnchor(TagNode node) {
+    return node.equalsElementName("A");
   }
 
   private static boolean isInsecureUrl(TagNode node) {

sonar-html-plugin/src/main/resources/org/sonar/l10n/web/rules/Web/S5148.html

@@ -8,28 +8,24 @@ <h2>Ask Yourself Whether</h2>
   <li> The application opens untrusted external URL. </li>
 </ul>
 <p>There is a risk if you answered yes to this question.</p>
-<h2>Recommended Secure Coding Practices</h2>
-<p>Use <code>noopener</code> to prevent untrusted pages from abusing <code>window.opener</code>.</p>
-<p>Note: In Chrome 88+, Firefox 79+ or Safari 12.1+ <code>target=_blank</code> on anchors implies <code>rel=noopener</code> which make the protection
-enabled by default.</p>
 <h2>Sensitive Code Example</h2>
 <pre>
-&lt;a href="http://example.com/dangerous" target="_blank"&gt; &lt;!-- Sensitive --&gt;
+&lt;a href="http://example.com/" rel="opener" target="_blank"&gt; &lt;!-- Sensitive --&gt;
 
-&lt;a href="{{variable}}" target="_blank"&gt; &lt;!-- Sensitive --&gt;
+&lt;a href="{{variable}}" rel="opener" target="_blank"&gt; &lt;!-- Sensitive --&gt;
 </pre>
 <h2>Compliant Solution</h2>
-<p>To prevent pages from abusing <code>window.opener</code>, use <code>rel=noopener</code> on <code>&lt;a href=&gt;</code> to force its value to be
-<code>null</code> on the opened pages.</p>
+<p>In Chrome 88+, Firefox 79+ or Safari 12.1+ <code>target=_blank</code> on anchors implies <code>rel=noopener</code> which makes the protection
+enabled by default.</p>
 <pre>
-&lt;a href="http://petssocialnetwork.io" target="_blank" rel="noopener"&gt;
+&lt;a href="https://example.com/" target="_blank" &gt;
 </pre>
 <h2>Exceptions</h2>
 <p>No Issue will be raised when <code>href</code> contains a hardcoded relative url as there it has less chances of being vulnerable. An url is
 considered hardcoded and relative if it doesn’t start with <code>http://</code> or <code>https://</code>, and if it does not contain any of the
 characters {}$()[]</p>
 <pre>
-&lt;a href="internal.html" target="_blank" &gt;
+&lt;a href="internal.html" rel="opener" target="_blank" &gt;
 </pre>
 <h2>See</h2>
 <ul>

sonar-html-plugin/src/test/java/org/sonar/plugins/html/checks/sonar/LinkWithTargetBlankCheckTest.java

@@ -32,10 +32,10 @@ public class LinkWithTargetBlankCheckTest {
   public void detected() throws Exception {
     HtmlSourceCode sourceCode = TestHelper.scan(new File("src/test/resources/checks/LinkWithTargetBlank.html"), new LinkWithTargetBlankCheck());
     checkMessagesVerifier.verify(sourceCode.getIssues())
-      .next().atLocation(1, 0, 1, 54).withMessage("Make sure not using rel=\"noopener\" is safe here.")
+      .next().atLocation(1, 0, 1, 67).withMessage("Make sure using target=\"_blank\" and rel=\"opener\" is safe here.")
       .next().atLine(2)
-      .next().atLine(7)
-      .next().atLine(9);
+      .next().atLine(3)
+      .next().atLine(8);
   }
 
 }

sonar-html-plugin/src/test/resources/checks/LinkWithTargetBlank.html

@@ -1,12 +1,14 @@
-<a href="http://dangerouswebsite.com" target="_blank">     <!-- Noncompliant; Make sure not using rel="noopener" is safe here. -->
-<a href="https://dangerouswebsite.com" target="_blank">     <!-- Noncompliant; Make sure not using rel="noopener" is safe here. -->
+<a href="http://dangerouswebsite.com" target="_blank" rel="opener">     <!-- Noncompliant; Make sure using target="_blank" and rel="opener" is safe here. -->
+<a href="https://dangerouswebsite.com" target="_blank" rel="opener">     <!-- Noncompliant; Make sure using target="_blank" and rel="opener" is safe here. -->
+<a href="https://dangerouswebsite.com" target="_blank" rel="whatever opener">     <!-- Noncompliant; Make sure using target="_blank" and rel="opener" is safe here. -->
 
 <a href="http://dangerouswebsite.com" target="_blank" rel="noopener noreferrer"> <!-- Compliant -->
 <a href="http://dangerouswebsite.com" target="_blank" rel="noopener">   <!-- Compliant -->
 
-<a href="{{variable}}" target="_blank" rel="noreferrer"> <!-- Noncompliant; Make sure not using rel="noopener" is safe here.  -->
+<a href="{{variable}}" target="_blank" rel="opener">  <!-- Noncompliant; Make sure using target="_blank" and rel="opener" is safe here. -->
+<a href="{{variable}}" target="_blank" rel="noreferrer"> <!-- Compliant  -->
 <a href="{{variable}}" target="_blank" rel="noopener"> <!-- Compliant  -->
-<a href="{{variable}}" target="_blank"> <!-- Noncompliant  -->
+<a href="{{variable}}" target="_blank"> <!-- Compliant  -->
 <a href="/{{variable}}" target="_blank"> <!-- Compliant  -->
 <a href="./{{variable}}" target="_blank"> <!-- Compliant  -->
 <a href="../{{variable}}" target="_blank"> <!-- Compliant  -->