You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2017/01/05 11:29:26 UTC
svn commit: r1777451 - in /ofbiz/trunk:
applications/content/src/main/java/org/apache/ofbiz/content/content/
applications/order/src/main/java/org/apache/ofbiz/order/order/
applications/party/src/main/java/org/apache/ofbiz/party/content/
applications/pr...
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);
}
}
Re: svn commit: r1777451 - in /ofbiz/trunk:
applications/content/src/main/java/org/apache/ofbiz/content/content/
applications/order/src/main/java/org/apache/ofbiz/order/order/
applications/party/src/main/java/org/apache/ofbiz/party/content/
applications/pr...
Posted by Michael Brohl <mi...@ecomify.de>.
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="<@o...@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="<@o...@ofbizUrl>">weitere Informationen</a>
</div>
</div>
</div>
will be rendered to
<div>
<img
src="<@o...@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="<@o...@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 jleroux@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);
> }
> }
>
>