Commit e0f67901 authored by Michael Hamann's avatar Michael Hamann
Browse files

XRENDERING-663: Restrict allowed attributes in HTML rendering

* Add tests for prefixed attributes.
* Move prefix removal to WikiModel and add duplicate attribute handling
  to ensure that, e.g., translated href attributes are correctly handled
  in WikiModel.
parent c40e2f5f
.#-----------------------------------------------------
.input|xwiki/2.1
.# Test JavaScript links.
.#-----------------------------------------------------
[[Click!>>path:javascript:alert(1)]]
.#-----------------------------------------------------
.expect|event/1.0
.#-----------------------------------------------------
beginDocument
beginParagraph
beginLink [Typed = [true] Type = [path] Reference = [javascript:alert(1)]] [false]
onWord [Click]
onSpecialSymbol [!]
endLink [Typed = [true] Type = [path] Reference = [javascript:alert(1)]] [false]
endParagraph
endDocument
.#-----------------------------------------------------
.expect|xhtml/1.0
.#-----------------------------------------------------
<p><span class="wikiinternallink"><a data-xwiki-translated-attribute-href="javascript:alert(1)">Click!</a></span></p>
.#-----------------------------------------------------
.expect|annotatedxhtml/1.0
.#-----------------------------------------------------
<p><!--startwikilink:true|-|path|-|javascript:alert(1)--><span class="wikiinternallink"><a data-xwiki-translated-attribute-href="javascript:alert(1)">Click!</a></span><!--stopwikilink--></p>
.#-----------------------------------------------------
.expect|xwiki/2.1
.#-----------------------------------------------------
[[Click!>>path:javascript:alert(1)]]
.#-----------------------------------------------------
.input|xhtml/1.0
.#-----------------------------------------------------
<p><!--startwikilink:true|-|path|-|javascript:alert(1)--><span class="wikiinternallink"><a data-xwiki-translated-attribute-href="javascript:alert(1)">Click!</a></span><!--stopwikilink--></p>
.#-----------------------------------------------------
.expect|plain/1.0
.#-----------------------------------------------------
Click!
\ No newline at end of file
......@@ -19,12 +19,10 @@
*/
package org.xwiki.rendering.internal.parser.xhtml.wikimodel;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.commons.lang3.tuple.ImmutablePair;
import org.apache.commons.lang3.tuple.Pair;
import org.xwiki.rendering.internal.parser.wikimodel.DefaultXWikiGeneratorListener;
import org.xwiki.rendering.listener.Listener;
......@@ -34,7 +32,6 @@
import org.xwiki.rendering.parser.ResourceReferenceParser;
import org.xwiki.rendering.parser.StreamParser;
import org.xwiki.rendering.renderer.PrintRendererFactory;
import org.xwiki.rendering.renderer.printer.XHTMLWikiPrinter;
import org.xwiki.rendering.syntax.Syntax;
import org.xwiki.rendering.util.IdGenerator;
import org.xwiki.rendering.wikimodel.WikiFormat;
......@@ -291,47 +288,4 @@ public void endFormat(WikiFormat format)
super.endFormat(format);
}
}
@Override
protected Map<String, String> convertParameters(WikiParameters params)
{
return maybeRemoveAttributePrefix(super.convertParameters(params));
}
@Override
protected Pair<Map<String, String>, Map<String, String>> convertAndSeparateParameters(WikiParameters params)
{
Pair<Map<String, String>, Map<String, String>> result = super.convertAndSeparateParameters(params);
Map<String, String> genericParameters = result.getRight();
if (!genericParameters.isEmpty()) {
genericParameters = maybeRemoveAttributePrefix(genericParameters);
}
return new ImmutablePair<>(result.getLeft(), genericParameters);
}
private Map<String, String> maybeRemoveAttributePrefix(Map<String, String> attributes)
{
Map<String, String> result;
if (!attributes.isEmpty()) {
result = new LinkedHashMap<>();
for (Map.Entry<String, String> entry : attributes.entrySet()) {
String newKey;
if (entry.getKey().startsWith(XHTMLWikiPrinter.TRANSLATED_ATTRIBUTE_PREFIX)) {
newKey = entry.getKey().substring(XHTMLWikiPrinter.TRANSLATED_ATTRIBUTE_PREFIX.length());
} else {
newKey = entry.getKey();
}
result.put(newKey, entry.getValue());
}
} else {
result = attributes;
}
return result;
}
}
.#-----------------------------------------------------
.input|xhtml/1.0
.# Testing script URL escaping
.#-----------------------------------------------------
<p>one <a href="javascript:alert(1)">two</a> three</p>
.#-----------------------------------------------------
.expect|event/1.0
.#-----------------------------------------------------
beginDocument
beginParagraph
onWord [one]
onSpace
beginLink [Typed = [true] Type = [path] Reference = [javascript:alert(1)]] [false]
onWord [two]
endLink [Typed = [true] Type = [path] Reference = [javascript:alert(1)]] [false]
onSpace
onWord [three]
endParagraph
endDocument
.#-----------------------------------------------------
.expect|xhtml/1.0
.#-----------------------------------------------------
<p>one <span class="wikiinternallink"><a data-xwiki-translated-attribute-href="javascript:alert(1)">two</a></span> three</p>
.#-----------------------------------------------------
.input|xhtml/1.0
.#-----------------------------------------------------
<p>one <a data-xwiki-translated-attribute-href="javascript:alert(1)">two</a> three</p>
\ No newline at end of file
.#-----------------------------------------------------
.input|xhtml/1.0
.# Testing duplicate translated attributes.
.#-----------------------------------------------------
<p>one <a data-xwiki-translated-attribute-href="translated" href="javascript:alert(1)">two</a> three</p>
.#-----------------------------------------------------
.expect|event/1.0
.#-----------------------------------------------------
beginDocument
beginParagraph
onWord [one]
onSpace
beginLink [Typed = [true] Type = [path] Reference = [javascript:alert(1)]] [false]
onWord [two]
endLink [Typed = [true] Type = [path] Reference = [javascript:alert(1)]] [false]
onSpace
onWord [three]
endParagraph
endDocument
.#-----------------------------------------------------
.expect|xhtml/1.0
.#-----------------------------------------------------
<p>one <span class="wikiinternallink"><a data-xwiki-translated-attribute-href="javascript:alert(1)">two</a></span> three</p>
\ No newline at end of file
......@@ -50,6 +50,11 @@
<artifactId>xwiki-commons-xml</artifactId>
<version>${commons.version}</version>
</dependency>
<dependency>
<groupId>org.xwiki.rendering</groupId>
<artifactId>xwiki-rendering-xml</artifactId>
<version>${project.version}</version>
</dependency>
<!-- Testing dependencies -->
<dependency>
......
......@@ -21,14 +21,18 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import org.xml.sax.Attributes;
import org.xml.sax.SAXException;
import org.xml.sax.ext.Attributes2;
import org.xml.sax.ext.LexicalHandler;
import org.xml.sax.helpers.DefaultHandler;
import org.xwiki.rendering.renderer.printer.XHTMLWikiPrinter;
import org.xwiki.rendering.wikimodel.WikiParameter;
import org.xwiki.rendering.wikimodel.WikiParameters;
import org.xwiki.rendering.wikimodel.impl.WikiScannerContext;
......@@ -186,8 +190,7 @@ public void endDocument() throws SAXException
}
/**
* @see org.xml.sax.helpers.DefaultHandler#endElement(java.lang.String,
* java.lang.String, java.lang.String)
* @see org.xml.sax.helpers.DefaultHandler#endElement(java.lang.String, java.lang.String, java.lang.String)
*/
@Override
public void endElement(String uri, String localName, String qName)
......@@ -206,8 +209,8 @@ public void startDocument() throws SAXException
}
/**
* @see org.xml.sax.helpers.DefaultHandler#startElement(java.lang.String,
* java.lang.String, java.lang.String, org.xml.sax.Attributes)
* @see org.xml.sax.helpers.DefaultHandler#startElement(java.lang.String, java.lang.String, java.lang.String,
* org.xml.sax.Attributes)
*/
@Override
public void startElement(
......@@ -273,9 +276,11 @@ private String getLocalName(
private WikiParameters getParameters(Attributes attributes)
{
Set<String> keys = new HashSet<>();
List<WikiParameter> params = new ArrayList<>();
for (int i = 0; i < attributes.getLength(); i++) {
String key = getLocalName(attributes.getQName(i), attributes.getLocalName(i), false);
keys.add(key);
String value = attributes.getValue(i);
WikiParameter param = new WikiParameter(key, value);
......@@ -301,6 +306,30 @@ private WikiParameters getParameters(Attributes attributes)
params.add(param);
}
}
return new WikiParameters(params);
List<WikiParameter> translatedParameters = params.stream()
// Remove prefixed attributes that also exist as non-prefixed version.
.filter(param -> {
// Kep all attributes without prefix.
boolean keep = !param.getKey().startsWith(XHTMLWikiPrinter.TRANSLATED_ATTRIBUTE_PREFIX);
if (!keep) {
String translatedKey =
param.getKey().substring(XHTMLWikiPrinter.TRANSLATED_ATTRIBUTE_PREFIX.length());
keep = !keys.contains(translatedKey);
}
return keep;
})
// Remove prefix.
.map(param -> {
if (param.getKey().startsWith(XHTMLWikiPrinter.TRANSLATED_ATTRIBUTE_PREFIX)) {
String translatedKey =
param.getKey().substring(XHTMLWikiPrinter.TRANSLATED_ATTRIBUTE_PREFIX.length());
return new WikiParameter(translatedKey, param.getValue());
} else {
return param;
}
})
.collect(Collectors.toList());
return new WikiParameters(translatedParameters);
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment