You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by mb...@apache.org on 2019/04/29 09:40:16 UTC
svn commit: r1858353 - in
/ofbiz/ofbiz-framework/branches/release18.12/framework/base: config/
src/main/java/org/apache/ofbiz/base/html/
src/main/java/org/apache/ofbiz/base/util/
Author: mbrohl
Date: Mon Apr 29 09:40:15 2019
New Revision: 1858353
URL: http://svn.apache.org/viewvc?rev=1858353&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/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/
ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java (with props)
ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java (with props)
Modified:
ofbiz/ofbiz-framework/branches/release18.12/framework/base/config/owasp.properties
ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java
Modified: ofbiz/ofbiz-framework/branches/release18.12/framework/base/config/owasp.properties
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release18.12/framework/base/config/owasp.properties?rev=1858353&r1=1858352&r2=1858353&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/branches/release18.12/framework/base/config/owasp.properties (original)
+++ ofbiz/ofbiz-framework/branches/release18.12/framework/base/config/owasp.properties Mon Apr 29 09:40:15 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/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java?rev=1858353&view=auto
==============================================================================
--- ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java (added)
+++ ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java Mon Apr 29 09:40:15 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/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java
------------------------------------------------------------------------------
svn:keywords = Date Rev Author URL Id
Propchange: ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java
------------------------------------------------------------------------------
svn:mime-type = text/plain
Added: ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java?rev=1858353&view=auto
==============================================================================
--- ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java (added)
+++ ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java Mon Apr 29 09:40:15 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/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java
------------------------------------------------------------------------------
svn:keywords = Date Rev Author URL Id
Propchange: ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java
------------------------------------------------------------------------------
svn:mime-type = text/plain
Modified: ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java?rev=1858353&r1=1858352&r2=1858353&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java (original)
+++ ofbiz/ofbiz-framework/branches/release18.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java Mon Apr 29 09:40:15 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;
@@ -88,20 +91,62 @@ 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
+ */
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.