Fix repeatedly adding rel values (#307)

* Don't allow duplicates in rel attribute for links Signed-off-by: Sven Strickroth <[email protected]> * Use lower case link rel attribute words --------- Signed-off-by: Sven Strickroth <[email protected]> Co-authored-by: Mike Samuel <[email protected]>
OWASP · Feb 2, 2024 · 2901ef0 · 2901ef0
1 parent 58fac00
commit 2901ef0
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 6 deletions.
diff --git a/src/main/java/org/owasp/html/HtmlPolicyBuilder.java b/src/main/java/org/owasp/html/HtmlPolicyBuilder.java
@@ -1045,12 +1045,11 @@ public String apply(String elementName, List<String> attrs) {
               for (int i = 0; i <= n; ++i) {
                 if (i == n || Strings.isHtmlSpace(rels.charAt(i))) {
                   if (left < i) {
-                    if (skip.isEmpty()
-                        || !skip.contains(
-                            Strings.toLowerCase(rels.substring(left, i)))) {
-                      String rel = rels.substring(left, i);
-                      present.add(rel);
-                      sb.append(rel).append(' ');
+                    final String rel = rels.substring(left, i);
+                    final String lowerCaseRel = Strings.toLowerCase(rel);
+                    if ((skip.isEmpty() || !skip.contains(lowerCaseRel)) && !present.contains(lowerCaseRel)) {
+                      present.add(lowerCaseRel);
+                      sb.append(lowerCaseRel).append(' ');
                     }
                   }
                   left = i + 1;

diff --git a/src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java b/src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java
@@ -924,6 +924,31 @@ public final void testRelLinksWhenRelIsPartOfData() {
 	  assertEquals(toSanitize, pf.sanitize(toSanitize));
   }
 
+  @Test
+  public static final void testRelLinksWithDuplicateRels() {
+    PolicyFactory pf = new HtmlPolicyBuilder()
+        .allowElements("a")
+        .allowAttributes("href").onElements("a")
+        .allowAttributes("rel").onElements("a")
+        .allowAttributes("target").onElements("a")
+        .allowStandardUrlProtocols()
+        .toFactory();
+    assertEquals("<a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://google.com\">test</a>", pf.sanitize("<a target=\"_blank\" rel=\"noopener noreferrer noreferrer\" href=\"https://google.com\">test</a>"));
+  }
+
+  @Test
+  public static final void testRelLinksWithDuplicateRelsRequired() {
+    PolicyFactory pf = new HtmlPolicyBuilder()
+        .allowElements("a")
+        .allowAttributes("href").onElements("a")
+        .allowAttributes("rel").onElements("a")
+        .allowAttributes("target").onElements("a")
+        .allowStandardUrlProtocols()
+        .requireRelsOnLinks("noreferrer")
+        .toFactory();
+    assertEquals("<a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://google.com\">test</a>", pf.sanitize("<a target=\"_blank\" rel=\"noopener noreferrer noreferrer\" href=\"https://google.com\">test</a>"));
+  }
+
   @Test
   public static final void testFailFastOnSpaceSeparatedStrings() {
     boolean failed;