{onclick=...} written by page authors to the rendered elements
+ * without any filtering, so without this extension any author could attach
+ * on* event handlers (stored XSS), style overlays or
+ * javascript: URLs to rendered content.
+ * {@code HtmlRenderer.ESCAPE_HTML} does not cover extension-assigned
+ * attributes.
+ *
+ * + * This extension must be registered after {@code AttributesExtension}, + * so its attribute provider runs last and sees the final attribute set of each + * node.
+ */ +public class AttributeSanitizerExtension implements HtmlRenderer.HtmlRendererExtension { + + /** + * Attributes whose value is a URL, and must therefore not carry a + * script-capable scheme. + */ + private static final Set< String> URL_ATTRIBUTES = Set.of("" + + "href", "src", "xlink:href", + "action", "formaction", + "background", "background-color", + "poster", "data", "cite"); + + public static AttributeSanitizerExtension create() { + return new AttributeSanitizerExtension(); + } + + /** + * {@inheritDoc} + */ + @Override + public void rendererOptions(final MutableDataHolder options) { + } + + /** + * {@inheritDoc} + */ + @Override + public void extend(final HtmlRenderer.Builder rendererBuilder, final String rendererType) { + rendererBuilder.attributeProviderFactory(new IndependentAttributeProviderFactory() { + + @Override + public AttributeProvider apply(final LinkResolverContext context) { + return AttributeSanitizerExtension::sanitize; + } + + }); + } + + /** + * Removes everyon* event handler, the style and
+ * srcdoc attributes, and any URL-valued attribute whose value
+ * carries a script-capable scheme.
+ *
+ * @param node the node being rendered
+ * @param part the attributable part of the node
+ * @param attributes the final attribute set of the node, mutated in place
+ */
+ static void sanitize(final Node node, final AttributablePart part, final MutableAttributes attributes) {
+ for (final String name : new ArrayList<>(attributes.keySet())) {
+ final String attribute = name.trim().toLowerCase(Locale.ENGLISH);
+ if (attribute.startsWith("on") || "style".equals(attribute) || "srcdoc".equals(attribute)) {
+ attributes.remove(name);
+ } else if (URL_ATTRIBUTES.contains(attribute) && hasForbiddenScheme(attributes.getValue(name))) {
+ attributes.remove(name);
+ }
+ }
+ }
+
+ static boolean hasForbiddenScheme(final String value) {
+ if (value == null) {
+ return false;
+ }
+ // browsers ignore ASCII control characters and whitespace when parsing URL schemes, so strip them first
+ final String url = value.toLowerCase(Locale.ENGLISH).replaceAll("[\\x00-\\x20]", "");
+ return url.startsWith("javascript:") || url.startsWith("vbscript:") || url.startsWith("data:");
+ }
+
+}
diff --git a/jspwiki-markdown/src/main/java/org/apache/wiki/parser/markdown/MarkdownDocument.java b/jspwiki-markdown/src/main/java/org/apache/wiki/parser/markdown/MarkdownDocument.java
index 639b686f87..abacd84bf1 100644
--- a/jspwiki-markdown/src/main/java/org/apache/wiki/parser/markdown/MarkdownDocument.java
+++ b/jspwiki-markdown/src/main/java/org/apache/wiki/parser/markdown/MarkdownDocument.java
@@ -33,6 +33,7 @@ Licensed to the Apache Software Foundation (ASF) under one
import org.apache.wiki.api.core.Context;
import org.apache.wiki.api.core.Page;
import org.apache.wiki.markdown.MarkdownForJSPWikiExtension;
+import org.apache.wiki.markdown.extensions.attributesanitizer.AttributeSanitizerExtension;
import org.apache.wiki.parser.MarkupParser;
import org.apache.wiki.parser.WikiDocument;
@@ -76,7 +77,10 @@ public static MutableDataSet options( final Context context, final boolean isIma
DefinitionExtension.create(),
FootnoteExtension.create(),
TablesExtension.create(),
- TocExtension.create() } ) );
+ TocExtension.create(),
+ // must be registered last, so its attribute provider sees (and can strip)
+ // the author-supplied attributes assigned by AttributesExtension
+ AttributeSanitizerExtension.create() } ) );
return options;
}
diff --git a/jspwiki-markdown/src/test/java/org/apache/wiki/render/markdown/AttributeSanitizerTest.java b/jspwiki-markdown/src/test/java/org/apache/wiki/render/markdown/AttributeSanitizerTest.java
new file mode 100644
index 0000000000..b1f95d0e2c
--- /dev/null
+++ b/jspwiki-markdown/src/test/java/org/apache/wiki/render/markdown/AttributeSanitizerTest.java
@@ -0,0 +1,87 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+package org.apache.wiki.render.markdown;
+
+import org.apache.wiki.HttpMockFactory;
+import org.apache.wiki.TestEngine;
+import org.apache.wiki.api.core.Context;
+import org.apache.wiki.api.core.Page;
+import org.apache.wiki.api.spi.Wiki;
+import org.apache.wiki.parser.markdown.MarkdownParser;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.io.BufferedReader;
+import java.io.StringReader;
+
+
+/**
+ * Regression tests for the markdown attribute sanitizer: author-written flexmark attribute lists must not be able
+ * to attach event handlers, styles or script-scheme URLs to rendered elements.
+ */
+public class AttributeSanitizerTest {
+
+ static final String PAGE_NAME = "attributesanitizertestpage";
+
+ TestEngine testEngine = TestEngine.build( TestEngine.with( "jspwiki.fileSystemProvider.pageDir", "./target/md-sanitizer-pageDir" ),
+ TestEngine.with( "jspwiki.renderingManager.markupParser", MarkdownParser.class.getName() ),
+ TestEngine.with( "jspwiki.renderingManager.renderer", MarkdownRenderer.class.getName() ) );
+
+ @Test
+ public void testEventHandlerAttributeIsStripped() throws Exception {
+ final String html = translate( "Click me{onmouseover=alert(document.cookie)}" );
+ Assertions.assertFalse( html.contains( "onmouseover" ), html );
+ Assertions.assertFalse( html.contains( "document.cookie" ), html );
+ }
+
+ @Test
+ public void testEventHandlerCaseVariationIsStripped() throws Exception {
+ final String html = translate( "Click me{ONCLICK=alert(1)}" );
+ Assertions.assertFalse( html.toLowerCase().contains( "onclick" ), html );
+ }
+
+ @Test
+ public void testStyleAttributeIsStripped() throws Exception {
+ final String html = translate( "Overlay{style=position:fixed;top:0;left:0;width:100vw;height:100vh}" );
+ Assertions.assertFalse( html.contains( "style=" ), html );
+ Assertions.assertFalse( html.contains( "position:fixed" ), html );
+ }
+
+ @Test
+ public void testJavascriptHrefIsStripped() throws Exception {
+ final String html = translate( "[link](x){href=javascript:alert(1)}" );
+ Assertions.assertFalse( html.contains( "javascript:" ), html );
+ }
+
+ @Test
+ public void testBenignAttributesSurvive() throws Exception {
+ final String html = translate( "Text{#anchor .highlight}" );
+ Assertions.assertTrue( html.contains( "anchor" ), html );
+ Assertions.assertTrue( html.contains( "highlight" ), html );
+ }
+
+ String translate( final String src ) throws Exception {
+ final Page page = Wiki.contents().page( testEngine, PAGE_NAME );
+ final Context context = Wiki.context().create( testEngine, HttpMockFactory.createHttpRequest(), page );
+ final MarkdownParser tr = new MarkdownParser( context, new BufferedReader( new StringReader( src ) ) );
+ final MarkdownRenderer conv = new MarkdownRenderer( context, tr.parse() );
+ return conv.getString();
+ }
+
+}
diff --git a/jspwiki-markdown/src/test/java/org/apache/wiki/render/markdown/MarkdownRendererTest.java b/jspwiki-markdown/src/test/java/org/apache/wiki/render/markdown/MarkdownRendererTest.java
index 2db4dd4e13..853a8d8112 100644
--- a/jspwiki-markdown/src/test/java/org/apache/wiki/render/markdown/MarkdownRendererTest.java
+++ b/jspwiki-markdown/src/test/java/org/apache/wiki/render/markdown/MarkdownRendererTest.java
@@ -149,7 +149,7 @@ public void testMarkupLinkWithCustomAttributes() throws Exception {
public void testMarkupPWithCustomAttributes() throws Exception {
// {..} are separated from the link, so they apply to the nearest p or span containing them
final String src0 = "This should be a [link](http://google.com) {style='background-color:#ddd'}";
- Assertions.assertEquals( "This should be a link
\n", translate( src0 ) ); + Assertions.assertEquals( "This should be a link
\n", translate( src0 ) ); final String src1 = "This should be a [link](http://google.com) {#a1}"; Assertions.assertEquals( "This should be a link
\n", translate( src1 ) );