CVE-2023-31126: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: `org.xwiki.commons:xwiki-commons-xml` is an XML library used by the open-source wiki platform XWiki. The HTML sanitizer, introduced in version 14.6-rc-1, allows the injection of arbitrary HTML code and thus cross-site scripting via invalid data attributes. This vulnerability does not affect restricted cleaning in HTMLCleaner as there attributes are cleaned and thus characters like `/` and `>` are removed in all attribute names. This problem has been patched in XWiki 14.10.4 and 15.0 RC1 by making sure that data attributes only contain allowed characters. There are no known workarounds apart from upgrading to a version including the fix.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 9.6
Published: 2023-05-09T13:15Z
Repository: https://github.com/xwiki/xwiki-commons
Language: Java
Files changed: xwiki-commons-core/xwiki-commons-xml/src/main/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizer.java, xwiki-commons-core/xwiki-commons-xml/src/test/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizerTest.java
Commit date: 2023-01-13 17:40:04 +0100
Commit message: XCOMMONS-2606: Properly validate data attributes in SecureHTMLElementSanitizer

* Make sure that the attribute is XML-compatible
* Add tests

Diff: diff --git a/xwiki-commons-core/xwiki-commons-xml/src/main/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizer.java b/xwiki-commons-core/xwiki-commons-xml/src/main/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizer.java
index 155c6d783b..ec6c1ba0c2 100644
--- a/xwiki-commons-core/xwiki-commons-xml/src/main/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizer.java
+++ b/xwiki-commons-core/xwiki-commons-xml/src/main/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizer.java
@@ -63,11 +63,22 @@ public class SecureHTMLElementSanitizer implements HTMLElementSanitizer, Initial
     static final Pattern IS_SCRIPT_OR_DATA = Pattern.compile("^(?:\\w+script|data):", Pattern.CASE_INSENSITIVE);
 
     static final Pattern ATTR_WHITESPACE =
         Pattern.compile("[\\u0000-\\u0020\\u00A0\\u1680\\u180E\\u2000-\\u2029\\u205F\\u3000]");
 
-    static final Pattern DATA_ATTR = Pattern.compile("^data-[\\-\\w.\\u00B7-\\uFFFF]");
+    /**
+     * Pattern that matches valid data-attributes.
+     * <p>
+     * Following the <a href="https://html.spec.whatwg.org/multipage/dom.html
+     #embedding-custom-non-visible-data-with-the-data-*-attributes">HTML standard</a>
+     * this means that the name starts with "data-", has at least one character after the hyphen and is
+     * <a href="https://html.spec.whatwg.org/multipage/infrastructure.html#xml-compatible">XML-compatible</a>,
+     * i.e., matches the <a href="https://www.w3.org/TR/xml/#NT-Name">Name production</a> without ":".
+     */
+    static final Pattern DATA_ATTR = Pattern.compile("^data-[A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6"
+        + "\\u00F8-\\u02ff\\u0370-\\u037d\\u037f-\\u1fff\\u200c\\u200d\\u2070-\\u218f\\u2c00-\\u2fef\\u3001-\\ud7ff"
+        + "\\uf900-\\ufdcf\\ufdf0-\\ufffd\\x{10000}-\\x{EFFFF}\\-.0-9\\u00b7\\u0300-\\u036f\\u203f-\\u2040]+$");
 
     static final Pattern ARIA_ATTR = Pattern.compile("^aria-[\\-\\w]+$");
 
     static final Pattern IS_ALLOWED_URI = Pattern.compile("^(?:(?:f|ht)tps?|mailto|tel|callto|cid|xmpp):",
         Pattern.CASE_INSENSITIVE);
@@ -180,11 +191,11 @@ public boolean isAttributeAllowed(String elementName, String attributeName, Stri
         boolean result = false;
 
         String lowerElement = elementName.toLowerCase();
         String lowerAttribute = attributeName.toLowerCase();
 
-        if ((DATA_ATTR.matcher(lowerAttribute).find() || ARIA_ATTR.matcher(lowerAttribute).find())
+        if ((DATA_ATTR.matcher(lowerAttribute).matches() || ARIA_ATTR.matcher(lowerAttribute).matches())
             && !this.forbidAttributes.contains(lowerAttribute))
         {
             result = true;
         } else if (isAttributeAllowed(lowerAttribute) && !this.forbidAttributes.contains(lowerAttribute)) {
             result = isAllowedValue(lowerElement, lowerAttribute, attributeValue);
diff --git a/xwiki-commons-core/xwiki-commons-xml/src/test/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizerTest.java b/xwiki-commons-core/xwiki-commons-xml/src/test/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizerTest.java
index 1722d0401c..e77fd864dd 100644
--- a/xwiki-commons-core/xwiki-commons-xml/src/test/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizerTest.java
+++ b/xwiki-commons-core/xwiki-commons-xml/src/test/java/org/xwiki/xml/internal/html/SecureHTMLElementSanitizerTest.java
@@ -21,17 +21,20 @@
 
 import java.util.Arrays;
 import java.util.Collections;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.xwiki.test.annotation.BeforeComponent;
 import org.xwiki.test.annotation.ComponentList;
 import org.xwiki.test.junit5.mockito.ComponentTest;
 import org.xwiki.test.junit5.mockito.InjectMockComponents;
 import org.xwiki.test.junit5.mockito.MockComponent;
 import org.xwiki.xml.html.HTMLConstants;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 
 /**
@@ -131,6 +134,22 @@ void restrictedURIs()
         assertTrue(this.secureHTMLElementSanitizer.isAttributeAllowed(HTMLConstants.TAG_A,
             HTMLConstants.ATTRIBUTE_HREF, "xwiki:test"));
         assertFalse(this.secureHTMLElementSanitizer.isAttributeAllowed(HTMLConstants.TAG_A,
             HTMLConstants.ATTRIBUTE_HREF, "http://example.com"));
     }
+
+    @ParameterizedTest
+    @CsvSource({
+        "data-, false",
+        "data-a, true",
+        "data-x-wiki.test_\u0192, true",
+        "data-x\u2713, false",
+        "data-x/test, false",
+        "data-x>test, false",
+        "data-x:y, false"
+    })
+    void dataAttributes(String attribute, boolean accepted)
+    {
+        assertEquals(accepted, this.secureHTMLElementSanitizer.isAttributeAllowed(HTMLConstants.TAG_DIV, attribute,
+            "hello"));
+    }
 }

Vulnerable code:     static final Pattern DATA_ATTR = Pattern.compile("^data-[\\-\\w.\\u00B7-\\uFFFF]");
        if ((DATA_ATTR.matcher(lowerAttribute).find() || ARIA_ATTR.matcher(lowerAttribute).find())
Fixed code:     /**
     * Pattern that matches valid data-attributes.
     * <p>
     * Following the <a href="https://html.spec.whatwg.org/multipage/dom.html
     #embedding-custom-non-visible-data-with-the-data-*-attributes">HTML standard</a>
     * this means that the name starts with "data-", has at least one character after the hyphen and is
     * <a href="https://html.spec.whatwg.org/multipage/infrastructure.html#xml-compatible">XML-compatible</a>,
     * i.e., matches the <a href="https://www.w3.org/TR/xml/#NT-Name">Name production</a> without ":".
     */
    static final Pattern DATA_ATTR = Pattern.compile("^data-[A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6"
        + "\\u00F8-\\u02ff\\u0370-\\u037d\\u037f-\\u1fff\\u200c\\u200d\\u2070-\\u218f\\u2c00-\\u2fef\\u3001-\\ud7ff"
        + "\\uf900-\\ufdcf\\ufdf0-\\ufffd\\x{10000}-\\x{EFFFF}\\-.0-9\\u00b7\\u0300-\\u036f\\u203f-\\u2040]+$");
        if ((DATA_ATTR.matcher(lowerAttribute).matches() || ARIA_ATTR.matcher(lowerAttribute).matches())
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import static org.junit.jupiter.api.Assertions.assertEquals;

    @ParameterizedTest
    @CsvSource({
        "data-, false",
        "data-a, true",
        "data-x-wiki.test_\u0192, true",
        "data-x\u2713, false",
        "data-x/test, false",
        "data-x>test, false",
        "data-x:y, false"
    })
    void dataAttributes(String attribute, boolean accepted)
    {
        assertEquals(accepted, this.secureHTMLElementSanitizer.isAttributeAllowed(HTMLConstants.TAG_DIV, attribute,
            "hello"));
    }