Re: svn commit: r1777451 - in /ofbiz/trunk: applications/content/src/main/
Locked
Re: svn commit: r1777451 - in /ofbiz/trunk: applications/content/src/main/
 
|
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 [hidden email]: > 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 (5K) | Free forum by Nabble | Edit this page |