Hi Jacques,this work breaks the proper rendering of html code. In our case, class attributes are stripped from the html content.
Example: <div class="item"><img src="<@ofbizContentUrl>/webcontent/img/slider/1.jpg</@ofbizContentUrl>" alt="" />
<div class="container"> <div class="slider-overlay"> <h2>Lorem ipsum dolor sit amet</h2> <h3>At vero eos et accusam et justo</h3> <p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.
</p><a class="btn btn-grey" href="<@ofbizUrl>cms/~webpage_id=100</@ofbizUrl>">weitere Informationen</a>
</div> </div> </div> will be rendered to <div><img src="<@ofbizContentUrl>/webcontent/img/slider/1.jpg</@ofbizContentUrl>" alt="" />
<div> <div> <h2>Lorem ipsum dolor sit amet</h2> <h3>At vero eos et accusam et justo</h3> <p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.
</p><a href="<@ofbizUrl>cms/~webpage_id=100</@ofbizUrl>">weitere Informationen</a>
</div> </div> </div>I do not see any reason to not allow class attributes in html code. Can you please have a look?
See https://issues.apache.org/jira/browse/OFBIZ-10187 Thanks, Michael Am 05.01.17 um 12:29 schrieb jler...@apache.org:
Author: jleroux Date: Thu Jan 5 11:29:26 2017 New Revision: 1777451 URL: http://svn.apache.org/viewvc?rev=1777451&view=rev Log: Implemented: Create and use an OWASP PolicyFactory for content sanitization in ContentWorker for Birt Report Builder (OFBIZ-9166) This is still an incomplete feature because I did not find yet a way to complete the BIRT_REPORT_BUILDER_USAGE_POLICY and had to bypass the sanitization for this case, WIP... This commit has no effect on OFBiz yet. I need to commit this now to continue to work on the main task: OFBIZ-6919 "New implementation of Birt. Easier user possibility of report creation". Modified: ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java ofbiz/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderContentWrapper.java ofbiz/trunk/applications/party/src/main/java/org/apache/ofbiz/party/content/PartyContentWrapper.java ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/category/CategoryContentWrapper.java ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigItemContentWrapper.java ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductContentWrapper.java ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductPromoContentWrapper.java ofbiz/trunk/applications/workeffort/src/main/java/org/apache/ofbiz/workeffort/content/WorkEffortContentWrapper.java ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java Modified: ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java (original) +++ ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java Thu Jan 5 11:29:26 2017 @@ -335,18 +335,22 @@ public class ContentWorker implements or String mimeTypeId, boolean cache) throws GeneralException, IOException { Writer writer = new StringWriter(); renderContentAsText(dispatcher, contentId, writer, templateContext, locale, mimeTypeId, null, null, cache); + 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 (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); + if (!"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); + } } return rendered; } Modified: ofbiz/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderContentWrapper.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderContentWrapper.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderContentWrapper.java (original) +++ ofbiz/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderContentWrapper.java Thu Jan 5 11:29:26 2017 @@ -111,7 +111,7 @@ public class OrderContentWrapper impleme if (UtilValidate.isEmpty(outString)) { outString = outString == null? "" : outString; } - outString = encoder.sanitize(outString); + outString = encoder.sanitize(outString, null); if (orderContentCache != null) { orderContentCache.put(cacheKey, outString); } Modified: ofbiz/trunk/applications/party/src/main/java/org/apache/ofbiz/party/content/PartyContentWrapper.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/main/java/org/apache/ofbiz/party/content/PartyContentWrapper.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/applications/party/src/main/java/org/apache/ofbiz/party/content/PartyContentWrapper.java (original) +++ ofbiz/trunk/applications/party/src/main/java/org/apache/ofbiz/party/content/PartyContentWrapper.java Thu Jan 5 11:29:26 2017 @@ -168,7 +168,7 @@ public class PartyContentWrapper impleme outString = party.getModelEntity().isField(candidateFieldName) ? party.getString(candidateFieldName): ""; outString = outString == null? "" : outString; } - outString = encoder.sanitize(outString); + outString = encoder.sanitize(outString, null); if (partyContentCache != null) { partyContentCache.put(cacheKey, outString); } @@ -176,11 +176,11 @@ public class PartyContentWrapper impleme } catch (GeneralException e) { Debug.logError(e, "Error rendering PartyContent, inserting empty String", module); String candidateOut = party.getModelEntity().isField(candidateFieldName) ? party.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } catch (IOException e) { Debug.logError(e, "Error rendering PartyContent, inserting empty String", module); String candidateOut = party.getModelEntity().isField(candidateFieldName) ? party.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } }Modified: ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/category/CategoryContentWrapper.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/category/CategoryContentWrapper.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/category/CategoryContentWrapper.java (original) +++ ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/category/CategoryContentWrapper.java Thu Jan 5 11:29:26 2017 @@ -110,7 +110,7 @@ public class CategoryContentWrapper impl outString = productCategory.getModelEntity().isField(candidateFieldName) ? productCategory.getString(candidateFieldName): ""; outString = outString == null? "" : outString; } - outString = encoder.sanitize(outString); + outString = encoder.sanitize(outString, null); if (categoryContentCache != null) { categoryContentCache.put(cacheKey, outString); } Modified: ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigItemContentWrapper.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigItemContentWrapper.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigItemContentWrapper.java (original) +++ ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigItemContentWrapper.java Thu Jan 5 11:29:26 2017 @@ -133,7 +133,7 @@ public class ProductConfigItemContentWra outString = productConfigItem.getModelEntity().isField(candidateFieldName) ? productConfigItem.getString(candidateFieldName): ""; outString = outString == null? "" : outString; } - outString = encoder.sanitize(outString); + outString = encoder.sanitize(outString, null); if (configItemContentCache != null) { configItemContentCache.put(cacheKey, outString); } @@ -141,11 +141,11 @@ public class ProductConfigItemContentWra } catch (GeneralException e) { Debug.logError(e, "Error rendering ProdConfItemContent, inserting empty String", module); String candidateOut = productConfigItem.getModelEntity().isField(candidateFieldName) ? productConfigItem.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } catch (IOException e) { Debug.logError(e, "Error rendering ProdConfItemContent, inserting empty String", module); String candidateOut = productConfigItem.getModelEntity().isField(candidateFieldName) ? productConfigItem.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } }Modified: ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductContentWrapper.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductContentWrapper.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductContentWrapper.java (original) +++ ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductContentWrapper.java Thu Jan 5 11:29:26 2017 @@ -123,7 +123,7 @@ public class ProductContentWrapper imple outString = product.getModelEntity().isField(candidateFieldName) ? product.getString(candidateFieldName): ""; outString = outString == null? "" : outString; } - outString = encoder.sanitize(outString); + outString = encoder.sanitize(outString, null); if (productContentCache != null) { productContentCache.put(cacheKey, outString); } @@ -131,11 +131,11 @@ public class ProductContentWrapper imple } catch (GeneralException e) { Debug.logError(e, "Error rendering ProductContent, inserting empty String", module); String candidateOut = product.getModelEntity().isField(candidateFieldName) ? product.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } catch (IOException e) { Debug.logError(e, "Error rendering ProductContent, inserting empty String", module); String candidateOut = product.getModelEntity().isField(candidateFieldName) ? product.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } }Modified: ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductPromoContentWrapper.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductPromoContentWrapper.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductPromoContentWrapper.java (original) +++ ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductPromoContentWrapper.java Thu Jan 5 11:29:26 2017 @@ -128,7 +128,7 @@ public class ProductPromoContentWrapper outString = productPromo.getModelEntity().isField(candidateFieldName) ? productPromo.getString(candidateFieldName): ""; outString = outString == null? "" : outString; } - outString = encoder.sanitize(outString); + outString = encoder.sanitize(outString, null); if (productPromoContentCache != null) { productPromoContentCache.put(cacheKey, outString); } @@ -136,11 +136,11 @@ public class ProductPromoContentWrapper } catch (GeneralException e) { Debug.logError(e, "Error rendering ProductPromoContent, inserting empty String", module); String candidateOut = productPromo.getModelEntity().isField(candidateFieldName) ? productPromo.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } catch (IOException e) { Debug.logError(e, "Error rendering ProductPromoContent, inserting empty String", module); String candidateOut = productPromo.getModelEntity().isField(candidateFieldName) ? productPromo.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } }Modified: ofbiz/trunk/applications/workeffort/src/main/java/org/apache/ofbiz/workeffort/content/WorkEffortContentWrapper.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/src/main/java/org/apache/ofbiz/workeffort/content/WorkEffortContentWrapper.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/applications/workeffort/src/main/java/org/apache/ofbiz/workeffort/content/WorkEffortContentWrapper.java (original) +++ ofbiz/trunk/applications/workeffort/src/main/java/org/apache/ofbiz/workeffort/content/WorkEffortContentWrapper.java Thu Jan 5 11:29:26 2017 @@ -256,7 +256,7 @@ public class WorkEffortContentWrapper im outString = workEffort.getModelEntity().isField(candidateFieldName) ? workEffort.getString(candidateFieldName): ""; outString = outString == null? "" : outString; } - outString = encoder.sanitize(outString); + outString = encoder.sanitize(outString, null); if (workEffortContentCache != null) { workEffortContentCache.put(cacheKey, outString); } @@ -264,11 +264,11 @@ public class WorkEffortContentWrapper im } catch (GeneralException e) { Debug.logError(e, "Error rendering WorkEffortContent, inserting empty String", module); String candidateOut = workEffort.getModelEntity().isField(candidateFieldName) ? workEffort.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } catch (IOException e) { Debug.logError(e, "Error rendering WorkEffortContent, inserting empty String", module); String candidateOut = workEffort.getModelEntity().isField(candidateFieldName) ? workEffort.getString(candidateFieldName): ""; - return candidateOut == null? "" : encoder.sanitize(candidateOut); + return candidateOut == null? "" : encoder.sanitize(candidateOut, null); } }Modified: ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java?rev=1777451&r1=1777450&r2=1777451&view=diff ============================================================================== --- ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java (original) +++ ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java Thu Jan 5 11:29:26 2017 @@ -60,7 +60,11 @@ public class UtilCodec {public static interface SimpleEncoder {public String encode(String original); - public String sanitize(String outString); // Only really useful with HTML, else simply calls encode() method + /** + * @deprecated Use {@link #sanitize(String,String)} instead + */ + public String sanitize(String outString); // Only really useful with HTML, else it simply calls encode() method + public String sanitize(String outString, String contentTypeId); // Only really useful with HTML, else it simply calls encode() method }public static interface SimpleDecoder {@@ -76,26 +80,70 @@ public class UtilCodec { } return htmlCodec.encode(IMMUNE_HTML, original); } + /** + * @deprecated Use {@link #sanitize(String,String)} instead + */ public String sanitize(String original) { + return sanitize(original, null); + } + public String sanitize(String original, String contentTypeId) { if (original == null) { return null; } PolicyFactory sanitizer = Sanitizers.FORMATTING.and(Sanitizers.BLOCKS).and(Sanitizers.IMAGES).and(Sanitizers.LINKS).and(Sanitizers.STYLES); - if (UtilProperties.getPropertyAsBoolean("owasp", "sanitizer.permissive.policy", false)) { + + if (UtilProperties.getPropertyAsBoolean("owasp", "sanitizer.permissive.policy", false)) {// TODO to be improved to use a (or several) contentTypeId/s if possible 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); + } 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 are <html> and <body> are not sanitized in 1st place (else the sanitizer makes some damages I found) + // 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... public static final PolicyFactory PERMISSIVE_POLICY = new HtmlPolicyBuilder() + .allowWithoutAttributes("html", "body") .allowAttributes("id", "class").globally() - .allowElements("html", "body", "div", "center", "span", "table", "td") + .allowElements("div", "center", "span", "table", "td") .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. ("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() + .allowWithoutAttributes("html", "body") + .allowElements("div", "span", "table", "tr", "td", "script") + .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", "onsubmit").onElements("form") + .toFactory(); }public static class XmlEncoder implements SimpleEncoder {@@ -107,7 +155,13 @@ public class UtilCodec { } return xmlCodec.encode(IMMUNE_XML, original); } + /** + * @deprecated Use {@link #sanitize(String,String)} instead + */ public String sanitize(String original) { + return sanitize(original, null); + } + public String sanitize(String original, String contentTypeId) { return encode(original); } } @@ -121,7 +175,13 @@ public class UtilCodec { return null; } } + /** + * @deprecated Use {@link #sanitize(String,String)} instead + */ public String sanitize(String original) { + return sanitize(original, null); + } + public String sanitize(String original, String contentTypeId) { return encode(original); }@@ -143,7 +203,13 @@ public class UtilCodec {} return original; } + /** + * @deprecated Use {@link #sanitize(String,String)} instead + */ public String sanitize(String original) { + return sanitize(original, null); + } + public String sanitize(String original, String contentTypeId) { return encode(original); } }
smime.p7s
Description: S/MIME Cryptographic Signature
