Author: jleroux
Date: Thu Apr 27 18:46:36 2017 New Revision: 1792939 URL: http://svn.apache.org/viewvc?rev=1792939&view=rev Log: Fixed: Create and use an OWASP PolicyFactory for content sanitization in ContentWorker for Birt Flexible Reports (OFBIZ-9166) Modified: ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java Modified: ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java?rev=1792939&r1=1792938&r2=1792939&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java (original) +++ ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java Thu Apr 27 18:46:36 2017 @@ -338,19 +338,17 @@ public class ContentWorker implements or GenericValue content = EntityQuery.use(dispatcher.getDelegator()).from("Content").where("contentId", contentId).queryOne(); String contentTypeId = content.getString("contentTypeId"); String rendered = writer.toString(); - // According to https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#XSS_Prevention_Rules_Summary - // Normally head is protected by X-XSS-Protection Response Header by default - if (!"FLEXIBLE_REPORT".equals(contentTypeId)) { // FIXME here BIRT_REPORT_BUILDER_USAGE_POLICY should be used but I could not tweak it yet: the content of <script> are removed and should not. Also a more annoying no yet spotted issue with contentId dissapearing - if (rendered.contains("<script>") - || rendered.contains("<!--") - || rendered.contains("<div") - || rendered.contains("<style>") - || rendered.contains("<span") - || rendered.contains("<input") - || rendered.contains("<iframe") - || rendered.contains("<a")) { - rendered = encoder.sanitize(rendered, contentTypeId); - } + // According to https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#XSS_Prevention_Rules_Summary, + // normally head is protected by X-XSS-Protection Response Header by default. + if (rendered.contains("<script>") + || rendered.contains("<!--") + || rendered.contains("<div") + || rendered.contains("<style>") + || rendered.contains("<span") + || rendered.contains("<input") + || rendered.contains("<iframe") + || rendered.contains("<a")) { + rendered = encoder.sanitize(rendered, contentTypeId); } return rendered; } Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java?rev=1792939&r1=1792938&r2=1792939&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java Thu Apr 27 18:46:36 2017 @@ -92,21 +92,22 @@ public class UtilCodec { } PolicyFactory sanitizer = Sanitizers.FORMATTING.and(Sanitizers.BLOCKS).and(Sanitizers.IMAGES).and(Sanitizers.LINKS).and(Sanitizers.STYLES); - if (UtilProperties.getPropertyAsBoolean("owasp", "sanitizer.permissive.policy", false)) {// TODO to be improved to use a (or several) contentTypeId/s if possible + // TODO to be improved to use a (or several) contentTypeId/s when necessary. Below is an example with BIRT_FLEXIBLE_REPORT_POLICY + if (UtilProperties.getPropertyAsBoolean("owasp", "sanitizer.permissive.policy", false)) { sanitizer = sanitizer.and(PERMISSIVE_POLICY); } - if ("REPORT_MASTER".equals(contentTypeId)) { - sanitizer = sanitizer.and(BIRT_REPORT_BUILDER_GENERATION_POLICY); - } - if ("REPORT".equals(contentTypeId)) { - sanitizer = sanitizer.and(BIRT_REPORT_BUILDER_USAGE_POLICY); + if ("FLEXIBLE_REPORT".equals(contentTypeId)) { + sanitizer = sanitizer.and(BIRT_FLEXIBLE_REPORT_POLICY); } return sanitizer.sanitize(original); } + // Given as an example based on rendering cmssite as it was before using the sanitizer. // To use the PERMISSIVE_POLICY set sanitizer.permissive.policy to true. // Note that I was unable to render </html> and </body>. I guess because <html> and <body> are not sanitized in 1st place (else the sanitizer makes some damages I found) // You might even want to adapt the PERMISSIVE_POLICY to your needs... Be sure to check https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet before... + // And https://github.com/OWASP/java-html-sanitizer/blob/master/docs/getting_started.md for examples. + // If you want another example: https://android.googlesource.com/platform/packages/apps/UnifiedEmail/+/ec0fa48/src/com/android/mail/utils/HtmlSanitizer.java public static final PolicyFactory PERMISSIVE_POLICY = new HtmlPolicyBuilder() .allowWithoutAttributes("html", "body") .allowAttributes("id", "class").globally() @@ -114,47 +115,32 @@ public class UtilCodec { .allowWithoutAttributes("html", "body", "div", "span", "table", "td") .allowAttributes("width").onElements("table") .toFactory(); - // This is the PolicyFactory used for the Birt Report Builder generation feature ("REPORT_MASTER" contentTypeId) - // It allows to create the OOTB Birt Report Builder example. - // You might need to enhance it for your needs but normally you should not - // In any case be sure to check https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet before changing things here... - public static final PolicyFactory BIRT_REPORT_BUILDER_GENERATION_POLICY = new HtmlPolicyBuilder() - .allowWithoutAttributes("html", "body") - .allowElements("div", "span", "table", "tr", "td") - .allowElements("form", "input", "textarea", "label", "select", "option") - .allowAttributes("id", "class", "name", "value", "onclick").globally() - .allowAttributes("width", "cellspacing").onElements("table") - .allowAttributes("type", "size", "maxlength").onElements("input") - .allowAttributes("cols", "rows").onElements("textarea") - .allowAttributes("class").onElements("td") - .allowAttributes("method").onElements("form") - .toFactory(); - // This is the PolicyFactory used for the Birt Report Builder usage feature. ("FLEXIBLE_REPORT" contentTypeId) + + // This is the PolicyFactory used for the Birt Report Builder usage feature. ("FLEXIBLE_REPORT" contentTypeId) // It allows to use the OOTB Birt Report Builder example. - // You might need to enhance it for your needs but normally you should not - // In any case be sure to check https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet before changing things here... - public static final PolicyFactory BIRT_REPORT_BUILDER_USAGE_POLICY = new HtmlPolicyBuilder() + // You might need to enhance it for your needs (when using a new REPORT_MASTER) but normally you should not. See PERMISSIVE_POLICY above for documentation and examples + public static final PolicyFactory BIRT_FLEXIBLE_REPORT_POLICY = new HtmlPolicyBuilder() .allowWithoutAttributes("html", "body") - .allowElements("div", "span", "table", "tr", "td", "script") - .allowElements("form", "input", "textarea", "label", "select", "option") + .allowElements("form", "div", "span", "table", "tr", "td", "input", "textarea", "label", "select", "option") .allowAttributes("id", "class", "name", "value", "onclick").globally() .allowAttributes("width", "cellspacing").onElements("table") .allowAttributes("type", "size", "maxlength").onElements("input") .allowAttributes("cols", "rows").onElements("textarea") .allowAttributes("class").onElements("td") - .allowAttributes("method", "onsubmit").onElements("form") + .allowAttributes("method").onElements("form") + .allowAttributes("accept", "action", "accept-charset", "autocomplete", "enctype", "method", "name", "novalidate", "target").onElements("form") .toFactory(); } - public static class XmlEncoder implements SimpleEncoder { - private static final char[] IMMUNE_XML = {',', '.', '-', '_', ' '}; - private XMLEntityCodec xmlCodec = new XMLEntityCodec(); - public String encode(String original) { - if (original == null) { - return null; - } - return xmlCodec.encode(IMMUNE_XML, original); - } + public static class XmlEncoder implements SimpleEncoder { + private static final char[] IMMUNE_XML = {',', '.', '-', '_', ' '}; + private XMLEntityCodec xmlCodec = new XMLEntityCodec(); + public String encode(String original) { + if (original == null) { + return null; + } + return xmlCodec.encode(IMMUNE_XML, original); + } /** * @deprecated Use {@link #sanitize(String,String)} instead */ |
Free forum by Nabble | Edit this page |