svn commit: r1858352 - in /ofbiz/ofbiz-framework/trunk/framework/base: config/ src/main/java/org/apache/ofbiz/base/html/ src/main/java/org/apache/ofbiz/base/util/

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

svn commit: r1858352 - in /ofbiz/ofbiz-framework/trunk/framework/base: config/ src/main/java/org/apache/ofbiz/base/html/ src/main/java/org/apache/ofbiz/base/util/

mbrohl
Author: mbrohl
Date: Mon Apr 29 09:24:13 2019
New Revision: 1858352

URL: http://svn.apache.org/viewvc?rev=1858352&view=rev
Log:
Fixed: OWASP sanitizer breaks proper rendering of HTML code
(OFBIZ-10187)

This makes the sanitizing enabled/disabled by configuration and enhances
the functionality to support custom sanitizer policies. A reasonable
example policy class is also included.

Thanks Dennis Balkir for reporting and providing the patch.

Added:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java   (with props)
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java   (with props)
Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/config/owasp.properties
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java

Modified: ofbiz/ofbiz-framework/trunk/framework/base/config/owasp.properties
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/config/owasp.properties?rev=1858352&r1=1858351&r2=1858352&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/config/owasp.properties (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/config/owasp.properties Mon Apr 29 09:24:13 2019
@@ -25,4 +25,8 @@
 # This has a slightly impact on the code rendered, see last comments in OFBIZ-6669.
 # Given as an example based on rendering cmssite, as it was before using the sanitizer.
 # 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...
-sanitizer.permissive.policy=false
+
+# Use sanitizer.permissive.policy=CUSTOM to use your custom PolicyFactory
+sanitizer.enable=true
+sanitizer.permissive.policy=DEFAULT
+sanitizer.custom.policy.class=org.apache.ofbiz.base.html.CustomPermissivePolicy
\ No newline at end of file

Added: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java?rev=1858352&view=auto
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java (added)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java Mon Apr 29 09:24:13 2019
@@ -0,0 +1,171 @@
+package org.apache.ofbiz.base.html;
+
+import java.util.regex.Pattern;
+
+import org.owasp.html.HtmlPolicyBuilder;
+import org.owasp.html.PolicyFactory;
+
+import com.google.common.base.Predicate;
+
+/**
+ * Based on the <a href=
+ * "http://www.owasp.org/index.php/Category:OWASP_AntiSamy_Project#Stage_2_-_Choosing_a_base_policy_file">AntiSamy
+ * EBay example</a>. <blockquote> eBay (http://www.ebay.com/) is the most
+ * popular online auction site in the universe, as far as I can tell. It is a
+ * public site so anyone is allowed to post listings with rich HTML content.
+ * It's not surprising that given the attractiveness of eBay as a target that it
+ * has been subject to a few complex XSS attacks. Listings are allowed to
+ * contain much more rich content than, say, Slashdot- so it's attack surface is
+ * considerably larger. The following tags appear to be accepted by eBay (they
+ * don't publish rules): {@code <a>},... </blockquote>
+ */
+public class CustomPermissivePolicy implements SanitizerCustomPolicy {
+
+    // Some common regular expression definitions.
+
+    // The 16 colors defined by the HTML Spec (also used by the CSS Spec)
+    private static final Pattern COLOR_NAME = Pattern.compile(
+            "(?:aqua|black|blue|fuchsia|gray|grey|green|lime|maroon|navy|olive|purple"
+                    + "|red|silver|teal|white|yellow)");
+
+    // HTML/CSS Spec allows 3 or 6 digit hex to specify color
+    private static final Pattern COLOR_CODE = Pattern.compile(
+            "(?:#(?:[0-9a-fA-F]{3}(?:[0-9a-fA-F]{3})?))");
+
+    private static final Pattern NUMBER_OR_PERCENT = Pattern.compile(
+            "[0-9]+%?");
+    private static final Pattern PARAGRAPH = Pattern.compile(
+            "(?:[\\p{L}\\p{N},'\\.\\s\\-_\\(\\)]|&[0-9]{2};)*");
+    private static final Pattern HTML_ID = Pattern.compile(
+            "[a-zA-Z0-9\\:\\-_\\.]+");
+    // force non-empty with a '+' at the end instead of '*'
+    private static final Pattern HTML_TITLE = Pattern.compile(
+            "[\\p{L}\\p{N}\\s\\-_',:\\[\\]!\\./\\\\\\(\\)&]*");
+    private static final Pattern HTML_CLASS = Pattern.compile(
+            "[a-zA-Z0-9\\s,\\-_]+");
+
+    private static final Pattern ONSITE_URL = Pattern.compile(
+            "(?:[\\p{L}\\p{N}\\\\\\.\\#@\\$%\\+&;\\-_~,\\?=/!]+|\\#(\\w)+)");
+    private static final Pattern OFFSITE_URL = Pattern.compile(
+            "\\s*(?:(?:ht|f)tps?://|mailto:)[\\p{L}\\p{N}]"
+                    + "[\\p{L}\\p{N}\\p{Zs}\\.\\#@\\$%\\+&;:\\-_~,\\?=/!\\(\\)]*+\\s*");
+
+    private static final Pattern NUMBER = Pattern.compile(
+            "[+-]?(?:(?:[0-9]+(?:\\.[0-9]*)?)|\\.[0-9]+)");
+
+    private static final Pattern NAME = Pattern.compile("[a-zA-Z0-9\\-_\\$]+");
+
+    private static final Pattern ALIGN = Pattern.compile(
+            "(?i)center|left|right|justify|char");
+
+    private static final Pattern VALIGN = Pattern.compile(
+            "(?i)baseline|bottom|middle|top");
+
+    private static final Predicate<String> COLOR_NAME_OR_COLOR_CODE = matchesEither(COLOR_NAME, COLOR_CODE);
+
+    private static final Predicate<String> ONSITE_OR_OFFSITE_URL = matchesEither(ONSITE_URL, OFFSITE_URL);
+
+    private static final Pattern HISTORY_BACK = Pattern.compile(
+            "(?:javascript:)?\\Qhistory.go(-1)\\E");
+
+    private static final Pattern ONE_CHAR = Pattern.compile(
+            ".?", Pattern.DOTALL);
+
+    /**
+     * A policy that can be used to produce policies that sanitize to HTML sinks via
+     * {@link PolicyFactory#apply}.
+     */
+    public static final PolicyFactory POLICY_DEFINITION = new HtmlPolicyBuilder()
+            .allowAttributes("id").matching(HTML_ID).globally()
+            .allowAttributes("class").matching(HTML_CLASS).globally()
+            .allowAttributes("lang").matching(Pattern.compile("[a-zA-Z]{2,20}"))
+            .globally()
+            .allowAttributes("title").matching(HTML_TITLE).globally()
+            .allowStyling()
+            .allowAttributes("align").matching(ALIGN).onElements("p")
+            .allowAttributes("for").matching(HTML_ID).onElements("label")
+            .allowAttributes("color").matching(COLOR_NAME_OR_COLOR_CODE)
+            .onElements("font")
+            .allowAttributes("face")
+            .matching(Pattern.compile("[\\w;, \\-]+"))
+            .onElements("font")
+            .allowAttributes("size").matching(NUMBER).onElements("font")
+            .allowAttributes("href").matching(ONSITE_OR_OFFSITE_URL)
+            .onElements("a")
+            .allowStandardUrlProtocols()
+            .allowAttributes("nohref").onElements("a")
+            .allowAttributes("target").matching(NAME).onElements("a")
+            .allowAttributes("name").matching(NAME).onElements("a")
+            .allowAttributes(
+                    "onfocus", "onblur", "onclick", "onmousedown", "onmouseup")
+            .matching(HISTORY_BACK).onElements("a")
+            .requireRelNofollowOnLinks()
+            .allowAttributes("src").matching(ONSITE_OR_OFFSITE_URL)
+            .onElements("img")
+            .allowAttributes("name").matching(NAME)
+            .onElements("img")
+            .allowAttributes("alt").matching(PARAGRAPH)
+            .onElements("img")
+            .allowAttributes("border", "hspace", "vspace").matching(NUMBER)
+            .onElements("img")
+            .allowAttributes("border", "cellpadding", "cellspacing")
+            .matching(NUMBER).onElements("table")
+            .allowAttributes("bgcolor").matching(COLOR_NAME_OR_COLOR_CODE)
+            .onElements("table")
+            .allowAttributes("align").matching(ALIGN)
+            .onElements("table")
+            .allowAttributes("noresize").matching(Pattern.compile("(?i)noresize"))
+            .onElements("table")
+            .allowAttributes("bgcolor").matching(COLOR_NAME_OR_COLOR_CODE)
+            .onElements("td", "th")
+            .allowAttributes("abbr").matching(PARAGRAPH)
+            .onElements("td", "th")
+            .allowAttributes("axis", "headers").matching(NAME)
+            .onElements("td", "th")
+            .allowAttributes("scope")
+            .matching(Pattern.compile("(?i)(?:row|col)(?:group)?"))
+            .onElements("td", "th")
+            .allowAttributes("nowrap")
+            .onElements("td", "th")
+            .allowAttributes("height", "width").matching(NUMBER_OR_PERCENT)
+            .onElements("table", "td", "th", "tr", "img")
+            .allowAttributes("align").matching(ALIGN)
+            .onElements("thead", "tbody", "tfoot", "img",
+                    "td", "th", "tr", "colgroup", "col")
+            .allowAttributes("valign").matching(VALIGN)
+            .onElements("thead", "tbody", "tfoot",
+                    "td", "th", "tr", "colgroup", "col")
+            .allowAttributes("charoff").matching(NUMBER_OR_PERCENT)
+            .onElements("td", "th", "tr", "colgroup", "col",
+                    "thead", "tbody", "tfoot")
+            .allowAttributes("char").matching(ONE_CHAR)
+            .onElements("td", "th", "tr", "colgroup", "col",
+                    "thead", "tbody", "tfoot")
+            .allowAttributes("colspan", "rowspan").matching(NUMBER)
+            .onElements("td", "th")
+            .allowAttributes("span", "width").matching(NUMBER_OR_PERCENT)
+            .onElements("colgroup", "col")
+            .allowElements(
+                    "a", "label", "noscript", "h1", "h2", "h3", "h4", "h5", "h6", "hr",
+                    "p", "i", "b", "u", "strong", "em", "small", "big", "pre", "code",
+                    "cite", "samp", "sub", "sup", "strike", "center", "blockquote",
+                    "hr", "br", "col", "font", "map", "span", "div", "img",
+                    "ul", "ol", "li", "dd", "dt", "dl", "tbody", "thead", "tfoot",
+                    "table", "td", "th", "tr", "colgroup", "fieldset", "legend", "header",
+                    "picture", "source", "section", "nav", "footer")
+            .toFactory();
+
+    private static Predicate<String> matchesEither(
+            final Pattern a, final Pattern b) {
+        return new Predicate<String>() {
+            public boolean apply(String s) {
+                return a.matcher(s).matches() || b.matcher(s).matches();
+            }
+        };
+    }
+
+    @Override
+    public PolicyFactory getSanitizerPolicy() {
+        return POLICY_DEFINITION;
+    }
+}

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java
------------------------------------------------------------------------------
    svn:keywords = Date Rev Author URL Id

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java?rev=1858352&view=auto
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java (added)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java Mon Apr 29 09:24:13 2019
@@ -0,0 +1,42 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *******************************************************************************/
+
+package org.apache.ofbiz.base.html;
+
+import org.owasp.html.HtmlPolicyBuilder;
+import org.owasp.html.PolicyFactory;
+
+/**
+ * This interface is used to build a custom sanitizer policy which then can be
+ * used instead of the default permissive policy. The custom policy will the be
+ * used in
+ * {@link org.apache.ofbiz.base.util.UtilCodec.HtmlEncoder#sanitize(String, String)}
+ */
+public interface SanitizerCustomPolicy {
+
+    public static final PolicyFactory POLICY_DEFINITION = new HtmlPolicyBuilder().toFactory();
+
+    /**
+     * Used for getting the policy from the custom class which implements this
+     * interface
+     *
+     * @return the policy specified in the class will be returned
+     */
+    PolicyFactory getSanitizerPolicy();
+}

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java
------------------------------------------------------------------------------
    svn:keywords = Date Rev Author URL Id

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

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=1858352&r1=1858351&r2=1858352&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 Mon Apr 29 09:24:13 2019
@@ -19,6 +19,8 @@
 package org.apache.ofbiz.base.util;
 
 import java.io.UnsupportedEncodingException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.net.URLDecoder;
 import java.net.URLEncoder;
 import java.util.ArrayList;
@@ -29,6 +31,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.ofbiz.base.html.SanitizerCustomPolicy;
 import org.owasp.esapi.codecs.Codec;
 import org.owasp.esapi.codecs.HTMLEntityCodec;
 import org.owasp.esapi.codecs.PercentCodec;
@@ -90,21 +93,63 @@ public class UtilCodec {
         public String sanitize(String original) {
             return sanitize(original, null);
         }
+
+        /**
+         * This method will start a configurable sanitizing process. The sanitizer can
+         * be turns off through "sanitizer.enable=false", the default value is true. It
+         * is possible to configure a custom policy using the properties
+         * "sanitizer.permissive.policy" and "sanitizer.custom.policy.class". The custom
+         * policy has to implement
+         * {@link org.apache.ofbiz.base.html.SanitizerCustomPolicy}.
+         *
+         * @param original
+         * @param contentTypeId
+         * @return sanitized HTML-Code if enabled, original HTML-Code when disabled
+         * @see org.apache.ofbiz.base.html.CustomPermissivePolicy
+         */
         @Override
         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.enable", true)) {
+                PolicyFactory sanitizer = Sanitizers.FORMATTING.and(Sanitizers.BLOCKS).and(Sanitizers.IMAGES).and(
+                        Sanitizers.LINKS).and(Sanitizers.STYLES);
+                // TODO to be improved to use a (or several) contentTypeId/s when necessary.
+                // Below is an example with BIRT_FLEXIBLE_REPORT_POLICY
+                if ("FLEXIBLE_REPORT".equals(contentTypeId)) {
+                    sanitizer = sanitizer.and(BIRT_FLEXIBLE_REPORT_POLICY);
+                }
+
+                // Check if custom policy should be used and if so don't use PERMISSIVE_POLICY
+                if ("CUSTOM".equals(UtilProperties.getPropertyValue("owasp", "sanitizer.permissive.policy"))) {
+                    PolicyFactory policy = null;
+                    try {
+                        Class<?> customPolicyClass = Class.forName(UtilProperties.getPropertyValue("owasp",
+                                "sanitizer.custom.policy.class"));
+                        Object obj = customPolicyClass.newInstance();
+                        if (SanitizerCustomPolicy.class.isAssignableFrom(customPolicyClass)) {
+                            Method meth = customPolicyClass.getMethod("getSanitizerPolicy");
+                            policy = (PolicyFactory) meth.invoke(obj);
+                        }
+                    } catch (ClassNotFoundException | IllegalAccessException | IllegalArgumentException
+                            | InvocationTargetException | NoSuchMethodException | SecurityException
+                            | InstantiationException e) {
+                        // Just logging the error and falling back to default policy
+                        Debug.logError(e, "Could not find custom sanitizer policy. Using default instead", module);
+                    }
+
+                    if (policy != null) {
+                        sanitizer = sanitizer.and(policy);
+                        return sanitizer.sanitize(original);
+                    }
+                }
 
-            // 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)) {
+                // Fallback should be the default option PERMISSIVE_POLICY
                 sanitizer = sanitizer.and(PERMISSIVE_POLICY);
+                return sanitizer.sanitize(original);
             }
-            if ("FLEXIBLE_REPORT".equals(contentTypeId)) {
-                sanitizer = sanitizer.and(BIRT_FLEXIBLE_REPORT_POLICY);
-            }
-            return sanitizer.sanitize(original);
+            return original;
         }
 
         // Given as an example based on rendering cmssite as it was before using the sanitizer.