diff --git a/src/main/java/org/olat/core/util/filter/impl/OWASPAntiSamyXSSFilter.java b/src/main/java/org/olat/core/util/filter/impl/OWASPAntiSamyXSSFilter.java index bea7128ddae15c69f05ae64f16cf09dbd342854d..51d85a3df85ef6e7b82846997836a6bd8c9684b8 100644 --- a/src/main/java/org/olat/core/util/filter/impl/OWASPAntiSamyXSSFilter.java +++ b/src/main/java/org/olat/core/util/filter/impl/OWASPAntiSamyXSSFilter.java @@ -19,11 +19,14 @@ */ package org.olat.core.util.filter.impl; +import java.io.IOException; import java.io.InputStream; import java.io.PrintWriter; +import java.io.StringReader; import java.io.StringWriter; import java.io.Writer; +import org.cyberneko.html.parsers.SAXParser; import org.olat.core.logging.OLATRuntimeException; import org.olat.core.logging.OLog; import org.olat.core.logging.Tracing; @@ -34,6 +37,10 @@ import org.owasp.validator.html.CleanResults; import org.owasp.validator.html.Policy; import org.owasp.validator.html.PolicyException; import org.owasp.validator.html.ScanException; +import org.xml.sax.Attributes; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.helpers.DefaultHandler; /** * Description:<br> @@ -137,7 +144,7 @@ public class OWASPAntiSamyXSSFilter implements Filter { ore.printStackTrace(printWriter); } - private String getCleanHTML(String original) { + private String getCleanHTML(String original) { Policy policy; if(variant == Variant.wiki) { if(entityEncodeIntlChars) { @@ -167,7 +174,10 @@ public class OWASPAntiSamyXSSFilter implements Filter { } catch (PolicyException e) { log.error("XSS Filter policy error", e); printOriginStackTrace(); - } + } catch (IllegalStateException e) { + //Bug in Batik with rgb values in percent: rgb(100%,20%,0%) + getCleanHTMLFromBatikBug(original, policy); + } String output; try { output = cr.getCleanHTML(); @@ -182,6 +192,37 @@ public class OWASPAntiSamyXSSFilter implements Filter { return output; } + private void getCleanHTMLFromBatikBug(String original, Policy policy) { + cr = null; + try { + String rgbCleanedOriginal = cleanHtml(original); + AntiSamy as = new AntiSamy(); + cr = as.scan(rgbCleanedOriginal, policy); + } catch (ScanException e) { + log.error("XSS Filter scan error", e); + printOriginStackTrace(); + } catch (PolicyException e) { + log.error("XSS Filter policy error", e); + printOriginStackTrace(); + } catch (IllegalStateException e) { + log.error("XSS Filter policy dramatic Batik error", e); + printOriginStackTrace(); + } + } + + private String cleanHtml(String original) { + try { + HTMLCleanerHandler handler = new HTMLCleanerHandler(); + SAXParser parser = new SAXParser(); + parser.setContentHandler(handler); + parser.parse(new InputSource(new StringReader(original))); + return handler.toString(); + } catch (SAXException | IOException e) { + log.error("", e); + return ""; + } + } + public int getNumOfErrors() { if (cr != null) { return cr.getNumberOfErrors(); @@ -191,7 +232,7 @@ public class OWASPAntiSamyXSSFilter implements Filter { /** * get Errors/Messages from filter. - * This have not to be "errors", its what has been filtered and gets reported. + * This have not to be "errors", its whatR has been filtered and gets reported. * @return */ public String getOrPrintErrorMessages(){ @@ -210,4 +251,63 @@ public class OWASPAntiSamyXSSFilter implements Filter { wiki } + + /** + * The handler will remove style attributes if it detects a RGB value + * to prevent: https://issues.apache.org/jira/browse/BATIK-1149<br> + * This is a bug in Batik which doesn't understand rgb values in percent. + * + * Initial date: 16 avr. 2019<br> + * @author srosse, stephane.rosse@frentix.com, http://www.frentix.com + * + */ + private static class HTMLCleanerHandler extends DefaultHandler { + + private final StringBuilder output = new StringBuilder(4096); + + @Override + public void startElement(String uri, String localName, String qName, Attributes attributes) { + output.append("<").append(localName); + int numOfAttributes = attributes.getLength(); + for(int i=0; i<numOfAttributes; i++) { + String attrName = attributes.getLocalName(i); + String attrValue = attributes.getValue(i); + if(attrValue.contains("rgb")) { + continue; + } + + output.append(' ').append(attrName).append("="); + boolean useSingle = attrValue.indexOf('"') >= 0; + if(useSingle) { + output.append('\''); + } else { + output.append('"'); + } + output.append(attrValue); + if(useSingle) { + output.append('\''); + } else { + output.append('"'); + } + } + output.append(">"); + } + + @Override + public void characters(char[] ch, int start, int length) throws SAXException { + if(output != null) { + output.append(ch, start, length); + } + } + + @Override + public void endElement(String uri, String localName, String qName) { + output.append("</").append(localName).append(">"); + } + + @Override + public String toString() { + return output.toString(); + } + } } diff --git a/src/test/java/org/olat/core/util/filter/impl/XSSFilterTest.java b/src/test/java/org/olat/core/util/filter/impl/XSSFilterTest.java index 359302959457adb4b170e52abf72c10e6f35c0e1..3f625724bf12634365da6343106130e921bb2ed4 100644 --- a/src/test/java/org/olat/core/util/filter/impl/XSSFilterTest.java +++ b/src/test/java/org/olat/core/util/filter/impl/XSSFilterTest.java @@ -235,6 +235,16 @@ public class XSSFilterTest { // t("<span style=\"font-family: serif, arial;\">preformated</span>", "<span style=\"font-family: courier new , courier;\">preformated</span>"); t("<span class=\"schoen\">irgendwas</span>", "<span class=\"schoen\">irgendwas</span>"); } + + /** + * This checks a bug in Batik + */ + @Test + public void test_style_rgb(){ + t("<p style=\"background-color: rgb(0%,0,0);\">background</p>", "<p>background</p>"); + t("<p style=\"background-color: rgba(100%,0,0);\">background</p>", "<p style=\"\">background</p>"); + t("<p style=\"background-color: rgb(100,50,50);\">background</p>", "<p style=\"background-color: rgb(100,50,50);\">background</p>"); + } @Test public void test_tiny_lists(){