svn commit: r1792939 - in /ofbiz/ofbiz-framework/trunk: applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1792939 - in /ofbiz/ofbiz-framework/trunk: applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java

jleroux@apache.org
Author: jleroux
Date: Thu Apr 27 18:46:36 2017
New Revision: 1792939

URL: http://svn.apache.org/viewvc?rev=1792939&view=rev
Log:
Fixed: Create and use an OWASP PolicyFactory for content sanitization in
ContentWorker for Birt Flexible Reports
(OFBIZ-9166)


Modified:
    ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java

Modified: ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java?rev=1792939&r1=1792938&r2=1792939&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java Thu Apr 27 18:46:36 2017
@@ -338,19 +338,17 @@ public class ContentWorker implements or
         GenericValue content = EntityQuery.use(dispatcher.getDelegator()).from("Content").where("contentId", contentId).queryOne();
         String contentTypeId = content.getString("contentTypeId");
         String rendered = writer.toString();
-        // According to https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#XSS_Prevention_Rules_Summary
-        // Normally head is protected by X-XSS-Protection Response Header by default
-        if (!"FLEXIBLE_REPORT".equals(contentTypeId)) { // FIXME here BIRT_REPORT_BUILDER_USAGE_POLICY should be used but I could not tweak it yet: the content of <script> are removed and should not. Also a more annoying no yet spotted issue with contentId dissapearing
-            if (rendered.contains("<script>")
-                    || rendered.contains("<!--")
-                    || rendered.contains("<div")
-                    || rendered.contains("<style>")
-                    || rendered.contains("<span")
-                    || rendered.contains("<input")
-                    || rendered.contains("<iframe")
-                    || rendered.contains("<a")) {
-                rendered = encoder.sanitize(rendered, contentTypeId);
-            }
+        // According to https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#XSS_Prevention_Rules_Summary,
+        // normally head is protected by X-XSS-Protection Response Header by default.
+        if (rendered.contains("<script>")
+                || rendered.contains("<!--")
+                || rendered.contains("<div")
+                || rendered.contains("<style>")
+                || rendered.contains("<span")
+                || rendered.contains("<input")
+                || rendered.contains("<iframe")
+                || rendered.contains("<a")) {
+            rendered = encoder.sanitize(rendered, contentTypeId);
         }
         return rendered;
     }

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java?rev=1792939&r1=1792938&r2=1792939&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java Thu Apr 27 18:46:36 2017
@@ -92,21 +92,22 @@ public class UtilCodec {
             }
             PolicyFactory sanitizer = Sanitizers.FORMATTING.and(Sanitizers.BLOCKS).and(Sanitizers.IMAGES).and(Sanitizers.LINKS).and(Sanitizers.STYLES);
 
-            if (UtilProperties.getPropertyAsBoolean("owasp", "sanitizer.permissive.policy", false)) {// TODO to be improved to use a (or several) contentTypeId/s if possible
+            // TODO to be improved to use a (or several) contentTypeId/s when necessary. Below is an example with BIRT_FLEXIBLE_REPORT_POLICY
+            if (UtilProperties.getPropertyAsBoolean("owasp", "sanitizer.permissive.policy", false)) {
                 sanitizer = sanitizer.and(PERMISSIVE_POLICY);
             }
-            if ("REPORT_MASTER".equals(contentTypeId)) {
-                sanitizer = sanitizer.and(BIRT_REPORT_BUILDER_GENERATION_POLICY);
-            }
-            if ("REPORT".equals(contentTypeId)) {
-                sanitizer = sanitizer.and(BIRT_REPORT_BUILDER_USAGE_POLICY);
+            if ("FLEXIBLE_REPORT".equals(contentTypeId)) {
+                sanitizer = sanitizer.and(BIRT_FLEXIBLE_REPORT_POLICY);
             }
             return sanitizer.sanitize(original);
         }
+        
         // Given as an example based on rendering cmssite as it was before using the sanitizer.
         // To use the PERMISSIVE_POLICY set sanitizer.permissive.policy to true.
         // Note that I was unable to render </html> and </body>. I guess because <html> and <body> are not sanitized in 1st place (else the sanitizer makes some damages I found)
         // You might even want to adapt the PERMISSIVE_POLICY to your needs... Be sure to check https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet before...
+        // And https://github.com/OWASP/java-html-sanitizer/blob/master/docs/getting_started.md for examples.
+        // If you want another example: https://android.googlesource.com/platform/packages/apps/UnifiedEmail/+/ec0fa48/src/com/android/mail/utils/HtmlSanitizer.java
         public static final PolicyFactory PERMISSIVE_POLICY = new HtmlPolicyBuilder()
                 .allowWithoutAttributes("html", "body")
                 .allowAttributes("id", "class").globally()
@@ -114,47 +115,32 @@ public class UtilCodec {
                 .allowWithoutAttributes("html", "body", "div", "span", "table", "td")
                 .allowAttributes("width").onElements("table")
                 .toFactory();
-        // This is the PolicyFactory used for the Birt Report Builder generation feature ("REPORT_MASTER" contentTypeId)
-        // It allows to create the OOTB Birt Report Builder example.
-        // You might need to enhance it for your needs but normally you should not
-        // In any case be sure to check https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet before changing things here...
-        public static final PolicyFactory BIRT_REPORT_BUILDER_GENERATION_POLICY = new HtmlPolicyBuilder()
-                .allowWithoutAttributes("html", "body")
-                .allowElements("div", "span", "table", "tr", "td")
-                .allowElements("form", "input", "textarea", "label", "select", "option")
-                .allowAttributes("id", "class", "name", "value", "onclick").globally()
-                .allowAttributes("width", "cellspacing").onElements("table")
-                .allowAttributes("type", "size", "maxlength").onElements("input")
-                .allowAttributes("cols", "rows").onElements("textarea")
-                .allowAttributes("class").onElements("td")
-                .allowAttributes("method").onElements("form")
-                .toFactory();
-        // This is the PolicyFactory used for the Birt Report Builder usage feature.  ("FLEXIBLE_REPORT" contentTypeId)
+        
+        // This is the PolicyFactory used for the Birt Report Builder usage feature. ("FLEXIBLE_REPORT" contentTypeId)
         // It allows to use the OOTB Birt Report Builder example.
-        // You might need to enhance it for your needs but normally you should not
-        // In any case be sure to check https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet before changing things here...
-        public static final PolicyFactory BIRT_REPORT_BUILDER_USAGE_POLICY = new HtmlPolicyBuilder()
+        // You might need to enhance it for your needs (when using a new REPORT_MASTER) but normally you should not. See PERMISSIVE_POLICY above for documentation and examples
+        public static final PolicyFactory BIRT_FLEXIBLE_REPORT_POLICY = new HtmlPolicyBuilder()
                 .allowWithoutAttributes("html", "body")
-                .allowElements("div", "span", "table", "tr", "td", "script")
-                .allowElements("form", "input", "textarea", "label", "select", "option")
+                .allowElements("form", "div", "span", "table", "tr", "td", "input", "textarea", "label", "select", "option")
                 .allowAttributes("id", "class", "name", "value", "onclick").globally()
                 .allowAttributes("width", "cellspacing").onElements("table")
                 .allowAttributes("type", "size", "maxlength").onElements("input")
                 .allowAttributes("cols", "rows").onElements("textarea")
                 .allowAttributes("class").onElements("td")
-                .allowAttributes("method", "onsubmit").onElements("form")
+                .allowAttributes("method").onElements("form")
+                .allowAttributes("accept", "action", "accept-charset", "autocomplete", "enctype", "method", "name", "novalidate", "target").onElements("form")
                 .toFactory();
     }
 
-    public static class XmlEncoder implements SimpleEncoder {
-        private static final char[] IMMUNE_XML = {',', '.', '-', '_', ' '};
-        private XMLEntityCodec xmlCodec = new XMLEntityCodec();
-        public String encode(String original) {
-            if (original == null) {
-                   return null;
-               }
-               return xmlCodec.encode(IMMUNE_XML, original);
-        }
+        public static class XmlEncoder implements SimpleEncoder {
+            private static final char[] IMMUNE_XML = {',', '.', '-', '_', ' '};
+            private XMLEntityCodec xmlCodec = new XMLEntityCodec();
+            public String encode(String original) {
+                if (original == null) {
+                    return null;
+                }
+                return xmlCodec.encode(IMMUNE_XML, original);
+            }
         /**
          * @deprecated Use {@link #sanitize(String,String)} instead
          */