Skip to content
Permalink
Browse files Browse the repository at this point in the history
XCOMMONS-2606: Properly validate data attributes in SecureHTMLElement…
…Sanitizer

* Make sure that the attribute is XML-compatible
* Add tests
  • Loading branch information
michitux committed Jan 13, 2023
1 parent 8c82fc6 commit 0b8e9c4
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
Expand Up @@ -65,7 +65,18 @@ public class SecureHTMLElementSanitizer implements HTMLElementSanitizer, Initial
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]+$");

Expand Down Expand Up @@ -182,7 +193,7 @@ public boolean isAttributeAllowed(String elementName, String attributeName, Stri
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;
Expand Down
Expand Up @@ -23,13 +23,16 @@
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;
Expand Down Expand Up @@ -133,4 +136,20 @@ void restrictedURIs()
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"));
}
}

0 comments on commit 0b8e9c4

Please sign in to comment.