diff --git a/its/ruling/src/test/resources/expected/Web-HeaderCheck.json b/its/ruling/src/test/resources/expected/Web-HeaderCheck.json index 45dd1df8..5c97e10e 100644 --- a/its/ruling/src/test/resources/expected/Web-HeaderCheck.json +++ b/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 ], diff --git a/its/ruling/src/test/resources/expected/Web-S1829.json b/its/ruling/src/test/resources/expected/Web-S1829.json index 67f46e93..55a46838 100644 --- a/its/ruling/src/test/resources/expected/Web-S1829.json +++ b/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, diff --git a/its/ruling/src/test/resources/expected/Web-S5148.json b/its/ruling/src/test/resources/expected/Web-S5148.json index 1b11a7d9..5f0e3813 100644 --- a/its/ruling/src/test/resources/expected/Web-S5148.json +++ b/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 ] } diff --git a/its/sources b/its/sources index 49de9f98..ef27916c 160000 --- a/its/sources +++ b/its/sources @@ -1 +1 @@ -Subproject commit 49de9f9885163a81e173eaecff48bce9f99546c3 +Subproject commit ef27916c27f4aae6e96e5c431a2cb7a5e144f153 diff --git a/sonar-html-plugin/src/main/java/org/sonar/plugins/html/checks/sonar/LinkWithTargetBlankCheck.java b/sonar-html-plugin/src/main/java/org/sonar/plugins/html/checks/sonar/LinkWithTargetBlankCheck.java index 40e1c504..f6f9336c 100644 --- a/sonar-html-plugin/src/main/java/org/sonar/plugins/html/checks/sonar/LinkWithTargetBlankCheck.java +++ b/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 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) { diff --git a/sonar-html-plugin/src/main/resources/org/sonar/l10n/web/rules/Web/S5148.html b/sonar-html-plugin/src/main/resources/org/sonar/l10n/web/rules/Web/S5148.html index 0a7df30a..8b4c3257 100644 --- a/sonar-html-plugin/src/main/resources/org/sonar/l10n/web/rules/Web/S5148.html +++ b/sonar-html-plugin/src/main/resources/org/sonar/l10n/web/rules/Web/S5148.html @@ -8,28 +8,24 @@

Ask Yourself Whether

  • The application opens untrusted external URL.

    • There is a risk if you answered yes to this question.

    -

    Recommended Secure Coding Practices

    -

    Use noopener to prevent untrusted pages from abusing window.opener.

    -

    Note: In Chrome 88+, Firefox 79+ or Safari 12.1+ target=_blank on anchors implies rel=noopener which make the protection -enabled by default.

    Sensitive Code Example

    -<a href="http://example.com/dangerous" target="_blank"> <!-- Sensitive -->
+<a href="http://example.com/" rel="opener" target="_blank"> <!-- Sensitive -->
 
-<a href="{{variable}}" target="_blank"> <!-- Sensitive -->
+<a href="{{variable}}" rel="opener" target="_blank"> <!-- Sensitive -->

    Compliant Solution

    -

    To prevent pages from abusing window.opener, use rel=noopener on <a href=> to force its value to be -null on the opened pages.

    +

    In Chrome 88+, Firefox 79+ or Safari 12.1+ target=_blank on anchors implies rel=noopener which makes the protection +enabled by default.

    -<a href="http://petssocialnetwork.io" target="_blank" rel="noopener">
+<a href="https://example.com/" target="_blank" >

    Exceptions

    No Issue will be raised when href 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 http:// or https://, and if it does not contain any of the characters {}$()[]

    -<a href="internal.html" target="_blank" >
+<a href="internal.html" rel="opener" target="_blank" >

    See