From e9b7ba0ef09ef3a7f409f6bca71b1911fe2e5065 Mon Sep 17 00:00:00 2001 From: vladbailescu Date: Mon, 17 Nov 2014 16:00:12 +0200 Subject: [PATCH] SLING-4176 - Added validation/filtering for StyleToken context --- .../engine/extension/XSSRuntimeExtension.java | 52 +++++------ .../java/org/apache/sling/xss/XSSAPI.java | 10 +++ .../org/apache/sling/xss/impl/XSSAPIImpl.java | 40 +++++++++ .../apache/sling/xss/impl/XSSAPIImplTest.java | 88 ++++++++++++++++--- 4 files changed, 151 insertions(+), 39 deletions(-) diff --git a/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/engine/extension/XSSRuntimeExtension.java b/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/engine/extension/XSSRuntimeExtension.java index d4438ea4b1..a4e84be7aa 100644 --- a/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/engine/extension/XSSRuntimeExtension.java +++ b/contrib/scripting/sightly/engine/src/main/java/org/apache/sling/scripting/sightly/engine/extension/XSSRuntimeExtension.java @@ -103,35 +103,29 @@ private String applyXSSFilter(String text, Object hint, MarkupContext xssContext } private String applyXSSFilter(String text, MarkupContext xssContext) { - if (xssContext.equals(MarkupContext.ATTRIBUTE)) { - return xssapi.encodeForHTMLAttr(text); - } - if (xssContext.equals(MarkupContext.COMMENT) || - xssContext.equals(MarkupContext.TEXT)) { - return xssapi.encodeForHTML(text); - } - if (xssContext.equals(MarkupContext.ATTRIBUTE_NAME)) { - return escapeAttributeName(text); - } - if (xssContext.equals(MarkupContext.NUMBER)) { - return xssapi.getValidLong(text, 0).toString(); - } - if (xssContext.equals(MarkupContext.URI)) { - return xssapi.getValidHref(text); - } - if (xssContext.equals(MarkupContext.SCRIPT_TOKEN) - || xssContext.equals(MarkupContext.SCRIPT_COMMENT)) { - return xssapi.getValidJSToken(text, ""); - } - if (xssContext.equals(MarkupContext.SCRIPT_STRING) - || xssContext.equals(MarkupContext.STYLE_STRING)) { - return xssapi.encodeForJSString(text); - } - if (xssContext.equals(MarkupContext.ELEMENT_NAME)) { - return escapeElementName(text); - } - if (xssContext.equals(MarkupContext.HTML)) { - return xssapi.filterHTML(text); + switch (xssContext) { + case ATTRIBUTE: + return xssapi.encodeForHTMLAttr(text); + case COMMENT: + case TEXT: + return xssapi.encodeForHTML(text); + case ATTRIBUTE_NAME: + return escapeAttributeName(text); + case NUMBER: + return xssapi.getValidLong(text, 0).toString(); + case URI: + return xssapi.getValidHref(text); + case SCRIPT_TOKEN: + case SCRIPT_COMMENT: + return xssapi.getValidJSToken(text, ""); + case STYLE_TOKEN: + return xssapi.getValidStyleToken(text, ""); + case SCRIPT_STRING: + return xssapi.encodeForJSString(text); + case ELEMENT_NAME: + return escapeElementName(text); + case HTML: + return xssapi.filterHTML(text); } return text; //todo: apply the rest of XSS filters } diff --git a/contrib/xss/src/main/java/org/apache/sling/xss/XSSAPI.java b/contrib/xss/src/main/java/org/apache/sling/xss/XSSAPI.java index b447324320..09801a6931 100644 --- a/contrib/xss/src/main/java/org/apache/sling/xss/XSSAPI.java +++ b/contrib/xss/src/main/java/org/apache/sling/xss/XSSAPI.java @@ -85,6 +85,16 @@ public interface XSSAPI { */ public String getValidJSToken(String token, String defaultValue); + /** + * Validate a style/CSS token. Valid CSS tokens are specified at http://www.w3.org/TR/css3-syntax/ + * + * @param token the source token + * @param defaultValue a default value to use if the source doesn't meet validity constraints. + * + * @return a string containing sanitized style token + */ + public String getValidStyleToken(String token, String defaultValue); + /** * Validate a CSS color value. Color values as specified at http://www.w3.org/TR/css3-color/#colorunits * are safe and definitively allowed. Vulnerable constructs will be disallowed. Currently known diff --git a/contrib/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java b/contrib/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java index 034a38df24..4306d9927d 100644 --- a/contrib/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java +++ b/contrib/xss/src/main/java/org/apache/sling/xss/impl/XSSAPIImpl.java @@ -190,6 +190,46 @@ public String getValidJSToken(String token, String defaultValue) { } } + private static final String NON_ASCII = "\\x00\\x08\\x0B\\x0C\\x0E-\\x1F"; + /** http://www.w3.org/TR/css-syntax-3/#number-token-diagram */ + private static final String NUMBER = "[+-]?[\\d]*[\\.]?[\\d]*(?:[e][+-]?\\d+)?"; + /** http://www.w3.org/TR/css-syntax-3/#hex-digit-diagram */ + private static final String HEX_DIGITS = "#[0-9a-f]*"; + /** http://www.w3.org/TR/css-syntax-3/#ident-token-diagram */ + private static final String IDENTIFIER = "-?[a-z_" + NON_ASCII + "][\\w_\\-" + NON_ASCII + "]*"; + /** http://www.w3.org/TR/css-syntax-3/#string-token-diagram */ + private static final String STRING = "\"(?:[^\"^\\\\^\\n]|(?:\\\\\"))*\"|'(?:[^'^\\\\^\\n]|(?:\\\\'))*'"; + /** http://www.w3.org/TR/css-syntax-3/#dimension-token-diagram */ + private static final String DIMENSION = NUMBER + IDENTIFIER; + /** http://www.w3.org/TR/css-syntax-3/#percentage-token-diagram */ + private static final String PERCENT = NUMBER + "%"; + /** http://www.w3.org/TR/css-syntax-3/#function-token-diagram */ + private static final String FUNCTION = IDENTIFIER + "\\((?:(?:" + NUMBER + ")|(?:" + IDENTIFIER + ")|(?:[\\s]*)|(?:,))*\\)"; + /** http://www.w3.org/TR/css-syntax-3/#url-unquoted-diagram */ + private static final String URL_UNQUOTED = "[^\"^'^\\(^\\)^[" + NON_ASCII + "]]*"; + /** http://www.w3.org/TR/css-syntax-3/#url-token-diagram */ + private static final String URL = "url\\((?:(?:" + URL_UNQUOTED + ")|(?:" + STRING + "))\\)"; + /** composite regular expression for style token validation */ + private static final String CSS_TOKEN = "(?i)" // case insensitive + + "(?:" + NUMBER + ")" + + "|(?:" + DIMENSION + ")" + + "|(?:" + PERCENT + ")" + + "|(?:" + HEX_DIGITS + ")" + + "|(?:" + IDENTIFIER + ")" + + "|(?:" + STRING + ")" + + "|(?:" + FUNCTION + ")" + + "|(?:" + URL + ")"; + + /** + * @see org.apache.sling.xss.XSSAPI#getValidStyleToken(String, String) + */ + public String getValidStyleToken(String token, String defaultValue) { + if (token.matches(CSS_TOKEN)) { + return token; + } + return defaultValue; + } + /** * @see org.apache.sling.xss.XSSAPI#getValidCSSColor(String, String) */ diff --git a/contrib/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java b/contrib/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java index 9637da6a8f..3f7108cba0 100644 --- a/contrib/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java +++ b/contrib/xss/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java @@ -39,6 +39,8 @@ public class XSSAPIImplTest { + public static final String RUBBISH = "rubbish"; + private XSSAPI xssAPI; @Before @@ -310,31 +312,97 @@ public void TestGetValidJSToken() { {"simple", "simple"}, {"clickstreamcloud.thingy", "clickstreamcloud.thingy"}, - {"break out", "rubbish"}, - {"break,out", "rubbish"}, + {"break out", RUBBISH}, + {"break,out", RUBBISH}, {"\"literal string\"", "\"literal string\""}, {"'literal string'", "'literal string'"}, - {"\"bad literal'", "rubbish"}, + {"\"bad literal'", RUBBISH}, {"'literal'); junk'", "'literal\\x27); junk'"}, {"1200", "1200"}, {"3.14", "3.14"}, - {"1,200", "rubbish"}, - {"1200 + 1", "rubbish"} + {"1,200", RUBBISH}, + {"1200 + 1", RUBBISH} }; for (String[] aTestData : testData) { String source = aTestData[0]; String expected = aTestData[1]; - String result = xssAPI.getValidJSToken(source, "rubbish"); + String result = xssAPI.getValidJSToken(source, RUBBISH); if (!result.equals(expected)) { fail("Validating Javascript token '" + source + "', expecting '" + expected + "', but got '" + result + "'"); } } } + @Test + public void TestGetValidStyleToken() { + String[][] testData = { + // Source Expected result + + // CSS close + {"}" , RUBBISH}, + + // line break + {"br\neak" , RUBBISH}, + + // no javascript: + {"javascript:alert(1)" , RUBBISH}, + {"url(javascript:alert(1))" , RUBBISH}, + + // no expression + {"expression(alert(1))" , RUBBISH}, + {"expression (alert(1))" , RUBBISH}, + {"expression(this.location='a.co')" , RUBBISH}, + + // html tags + {"", RUBBISH}, + + // usual CSS stuff + {"background-color" , "background-color"}, + {"-moz-box-sizing" , "-moz-box-sizing"}, + {".42%" , ".42%"}, + {"#fff" , "#fff"}, + + // valid strings + {"'literal string'" , "'literal string'"}, + {"\"literal string\"" , "\"literal string\""}, + {"'it\\'s here'" , "'it\\'s here'"}, + {"\"it\\\"s here\"" , "\"it\\\"s here\""}, + + // invalid strings + {"\"bad string" , RUBBISH}, + {"'it's here'" , RUBBISH}, + {"\"it\"s here\"" , RUBBISH}, + + // valid parenthesis + {"rgb(255, 255, 255)" , "rgb(255, 255, 255)"}, + + // invalid parenthesis + {"rgb(255, 255, 255" , RUBBISH}, + {"255, 255, 255)" , RUBBISH}, + + // valid tokens + {"url(http://example.com/test.png)", "url(http://example.com/test.png)"}, + {"url('image/test.png')" , "url('image/test.png')"}, + + // invalid tokens + {"color: red" , RUBBISH} + }; + + for (String[] aTestData : testData) { + String source = aTestData[0]; + String expected = aTestData[1]; + + String result = xssAPI.getValidStyleToken(source, RUBBISH); + if (!result.equals(expected)) { + fail("Validating style token '" + source + "', expecting '" + expected + "', but got '" + result + "'"); + } + } + } + @Test public void TestGetValidCSSColor() { String[][] testData = { @@ -352,16 +420,16 @@ public void TestGetValidCSSColor() { {"transparent", "transparent"}, {"\f\r\n\t MenuText\f\r\n\t ", "MenuText"}, - {"expression(99,99,99)", "rubbish"}, - {"blue;", "rubbish"}, - {"url(99,99,99)", "rubbish"} + {"expression(99,99,99)", RUBBISH}, + {"blue;", RUBBISH}, + {"url(99,99,99)", RUBBISH} }; for (String[] aTestData : testData) { String source = aTestData[0]; String expected = aTestData[1]; - String result = xssAPI.getValidCSSColor(source, "rubbish"); + String result = xssAPI.getValidCSSColor(source, RUBBISH); if (!result.equals(expected)) { fail("Validating CSS Color '" + source + "', expecting '" + expected + "', but got '" + result + "'"); }