svn commit: r1858354 - in /ofbiz/ofbiz-framework/branche
Locked
 
|
Author: mbrohl
Date: Mon Apr 29 09:42:55 2019 New Revision: 1858354 URL: http://svn.apache.org/viewvc?rev=1858354&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/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/ ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java (with props) ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java (with props) Modified: ofbiz/ofbiz-framework/branches/release17.12/framework/base/config/owasp.properties ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java Modified: ofbiz/ofbiz-framework/branches/release17.12/framework/base/config/owasp.properties URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/base/config/owasp.properties?rev=1858354&r1=1858353&r2=1858354&view=diff ============================================================================== --- ofbiz/ofbiz-framework/branches/release17.12/framework/base/config/owasp.properties (original) +++ ofbiz/ofbiz-framework/branches/release17.12/framework/base/config/owasp.properties Mon Apr 29 09:42:55 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/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java?rev=1858354&view=auto ============================================================================== --- ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java (added) +++ ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java Mon Apr 29 09:42:55 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/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: ofbiz/ofbiz-framework/branches/release17.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/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/CustomPermissivePolicy.java ------------------------------------------------------------------------------ svn:mime-type = text/plain Added: ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java?rev=1858354&view=auto ============================================================================== --- ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java (added) +++ ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java Mon Apr 29 09:42:55 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/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: ofbiz/ofbiz-framework/branches/release17.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/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/html/SanitizerCustomPolicy.java ------------------------------------------------------------------------------ svn:mime-type = text/plain Modified: ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java?rev=1858354&r1=1858353&r2=1858354&view=diff ============================================================================== --- ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java (original) +++ ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java Mon Apr 29 09:42:55 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; @@ -86,20 +89,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. |
«
Return to OFBiz - Commits
|
1 view|%1 views
| Free forum by Nabble | Edit this page |