JSPWIKI-1296 see jira for details

jspwiki-markdown/src/main/java/org/apache/wiki/markdown/extensions/attributesanitizer/AttributeSanitizerExtension.java

@@ -0,0 +1,117 @@
+/*
+    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.markdown.extensions.attributesanitizer;
+
+import com.vladsch.flexmark.html.AttributeProvider;
+import com.vladsch.flexmark.html.HtmlRenderer;
+import com.vladsch.flexmark.html.IndependentAttributeProviderFactory;
+import com.vladsch.flexmark.html.renderer.AttributablePart;
+import com.vladsch.flexmark.html.renderer.LinkResolverContext;
+import com.vladsch.flexmark.util.ast.Node;
+import com.vladsch.flexmark.util.data.MutableDataHolder;
+import com.vladsch.flexmark.util.html.MutableAttributes;
+
+import java.util.ArrayList;
+import java.util.Locale;
+import java.util.Set;
+
+/**
+ * {@link HtmlRenderer.HtmlRendererExtension} which strips dangerous
+ * author-supplied attributes from the rendered HTML. Flexmark's
+ * AttributesExtension applies attribute lists such as
+ * <code>{onclick=...}</code> written by page authors to the rendered elements
+ * without any filtering, so without this extension any author could attach
+ * <code>on*</code> event handlers (stored XSS), <code>style</code> overlays or
+ * <code>javascript:</code> URLs to rendered content.
+ * {@code HtmlRenderer.ESCAPE_HTML} does not cover extension-assigned
+ * attributes.
+ *
+ * <p>
+ * This extension must be registered <em>after</em> {@code AttributesExtension},
+ * so its attribute provider runs last and sees the final attribute set of each
+ * node.</p>
+ */
+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 every <code>on*</code> event handler, the <code>style</code> and
+     * <code>srcdoc</code> 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:");
+    }
+
+}

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;
     }
 

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();
+    }
+
+}

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( "<p style=\"background-color:#ddd\">This should be a <a href=\"http://google.com\" class=\"external\">link</a></p>\n", translate( src0 ) );
+        Assertions.assertEquals( "<p>This should be a <a href=\"http://google.com\" class=\"external\">link</a></p>\n", translate( src0 ) );
 
         final String src1 = "This should be a [link](http://google.com) {#a1}";
         Assertions.assertEquals( "<p id=\"a1\">This should be a <a href=\"http://google.com\" class=\"external\">link</a></p>\n", translate( src1 ) );