This is an automated email from the ASF dual-hosted git repository. jleroux pushed a commit to branch release18.12 in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release18.12 by this push: new 843b1c7e71 Improved: Prevent Freemarker interpolation in fields (OFBIZ-12594) 843b1c7e71 is described below commit 843b1c7e71aa046dc8205cb1cdf14011ca17aaa8 Author: Jacques Le Roux <jacques.le.r...@les7arts.com> AuthorDate: Mon Apr 4 08:04:39 2022 +0200 Improved: Prevent Freemarker interpolation in fields (OFBIZ-12594) OFBIZ_12587 is a definitive solution to prevent any kind of Freemarker exploits. But it's hard to realise because OFBiz exposes objects, like attributes from the Servlet scopes. So in the meantime preventing Freemarker interpolation in fields is a pragmatic solution. This is an improvement but needs to be backported because it kinda affects security Conflicts handled by hand SeoContextFilter.java ControlFilter.java When I worked with Mathieu I did not measure how it will be hard sometimes to backport later :/ --- .../ofbiz/product/category/SeoContextFilter.java | 10 ++- framework/security/config/security.properties | 4 ++ .../org/apache/ofbiz/security/SecurityUtil.java | 78 +++++++++++++++++++++- .../apache/ofbiz/webapp/control/ControlFilter.java | 7 ++ 4 files changed, 94 insertions(+), 5 deletions(-) diff --git a/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java b/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java index b518dedbad..71e7b82f0e 100644 --- a/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java +++ b/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java @@ -48,6 +48,7 @@ import org.apache.ofbiz.base.util.Debug; import org.apache.ofbiz.base.util.StringUtil; import org.apache.ofbiz.base.util.UtilHttp; import org.apache.ofbiz.base.util.UtilValidate; +import org.apache.ofbiz.security.SecurityUtil; import org.apache.ofbiz.webapp.control.ConfigXMLReader; import org.apache.ofbiz.webapp.control.ConfigXMLReader.ControllerConfig; import org.apache.ofbiz.webapp.control.ControlFilter; @@ -112,7 +113,11 @@ public class SeoContextFilter implements Filter { String queryString = URLEncodedUtils.format(params, Charset.forName("UTF-8")); uri = uri + "?" + queryString; } - + + if (SecurityUtil.containsFreemarkerInterpolation(httpRequest, httpResponse, uri)) { + return; + } + boolean forwarded = forwardUri(httpResponse, uri); if (forwarded) { return; @@ -167,7 +172,7 @@ public class SeoContextFilter implements Filter { if (pathItemList != null) { viewName = pathItemList.get(0); } - + String requestUri = UtilHttp.getRequestUriFromTarget(httpRequest.getRequestURI()); // check to make sure the requested url is allowed @@ -231,7 +236,6 @@ public class SeoContextFilter implements Filter { /** * Forward a uri according to forward pattern regular expressions. Note: this is developed for Filter usage. - * * @param uri String to reverse transform * @return String */ diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties index 6b12b01b01..352188f760 100644 --- a/framework/security/config/security.properties +++ b/framework/security/config/security.properties @@ -253,3 +253,7 @@ allowedProtocols=localhost,127.0.0.1 #-- By default (OOTB) OFBiz is protected against Large File Denial of Service because build.gradle defines -Xmx1024M #-- So you can at most upload a file around 500MB (see OFBIZ-11534 for more info) #-- If you need to upload larger files then follow https://nightlies.apache.org/ofbiz/trunk/readme/html5/#passing-jvm-runtime-options-to-ofbiz + +#-- Prevent Freemarker exploits +#-- eg: allowedURIsForFreemarkerInterpolation=createTextContentCms,updateTextContentCms,... +allowedURIsForFreemarkerInterpolation= diff --git a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java index 56f5e418df..046b67beba 100644 --- a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java +++ b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java @@ -19,13 +19,23 @@ package org.apache.ofbiz.security; +import java.io.IOException; +import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.stream.Collectors; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.http.client.utils.URLEncodedUtils; +import org.apache.http.message.BasicNameValuePair; import org.apache.ofbiz.base.util.Debug; import org.apache.ofbiz.base.util.StringUtil; +import org.apache.ofbiz.base.util.UtilHttp; import org.apache.ofbiz.base.util.UtilMisc; +import org.apache.ofbiz.base.util.UtilProperties; import org.apache.ofbiz.base.util.UtilValidate; import org.apache.ofbiz.entity.Delegator; import org.apache.ofbiz.entity.GenericEntityException; @@ -56,7 +66,7 @@ public final class SecurityUtil { * * @param delegator * @param userLoginId - * @return + * @return boolean */ public static boolean hasUserLoginAdminPermission(Delegator delegator, String userLoginId) { if (UtilValidate.isEmpty(userLoginId)) return false; @@ -80,7 +90,7 @@ public final class SecurityUtil { * @param delegator * @param userLoginId * @param toUserLoginId - * @return + * @return List<String> */ public static List<String> hasUserLoginMorePermissionThan(Delegator delegator, String userLoginId, String toUserLoginId) { ArrayList<String> returnList = new ArrayList<>(); @@ -165,4 +175,68 @@ public final class SecurityUtil { } return false; } + + /* + * Prevents Freemarker exploits + * @param req + * @param resp + * @param uri + * @throws IOException + */ + public static boolean containsFreemarkerInterpolation(HttpServletRequest req, HttpServletResponse resp, String uri) + throws IOException { + String urisOkForFreemarker = UtilProperties.getPropertyValue("security", "allowedURIsForFreemarkerInterpolation"); + List<String> urisOK = UtilValidate.isNotEmpty(urisOkForFreemarker) ? StringUtil.split(urisOkForFreemarker, ",") + : new ArrayList<>(); + String uriEnd = uri.substring(uri.lastIndexOf("/") + 1, uri.length()); + + if (!urisOK.contains(uriEnd)) { + Map<String, String[]> parameterMap = req.getParameterMap(); + if (uri.contains("ecomseo")) { // SeoContextFilter call + if (containsFreemarkerInterpolation(resp, uri)) { + return true; + } + } else if (!parameterMap.isEmpty()) { // ControlFilter call + List<BasicNameValuePair> params = new ArrayList<>(); + parameterMap.forEach((name, values) -> { + for (String value : values) { + params.add(new BasicNameValuePair(name, value)); + } + }); + String queryString = URLEncodedUtils.format(params, Charset.forName("UTF-8")); + uri = uri + "?" + queryString; + if (SecurityUtil.containsFreemarkerInterpolation(resp, uri)) { + return true; + } + } else if (!UtilHttp.getAttributeMap(req).isEmpty()) { // Call with Content-Type modified by a MITM attack (rare case) + String attributeMap = UtilHttp.getAttributeMap(req).toString(); + if (containsFreemarkerInterpolation(resp, attributeMap)) { + return true; + } + } + } + return false; + } + + /** + * @param resp + * @param stringToCheck + * @throws IOException + */ + public static boolean containsFreemarkerInterpolation(HttpServletResponse resp, String stringToCheck) throws IOException { + if (stringToCheck.contains("%24%7B") || stringToCheck.contains("${") + || stringToCheck.contains("%3C%23") || stringToCheck.contains("<#") + || stringToCheck.contains("%23%7B") || stringToCheck.contains("#{") + || stringToCheck.contains("%5B%3D") || stringToCheck.contains("[=") + || stringToCheck.contains("%5B%23") || stringToCheck.contains("[#")) { // not used OOTB in OFBiz, but possible + + Debug.logError("===== Not saved for security reason, strings '${', '<#', '#{', '[=' or '[#' not accepted in fields! =====", + MODULE); + resp.sendError(HttpServletResponse.SC_FORBIDDEN, + "Not saved for security reason, strings '${', '<#', '#{', '[=' or '[#' not accepted in fields!"); + return true; + } else { + return false; + } + } } diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java index 4714c10f2f..750a85658f 100644 --- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java +++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java @@ -32,6 +32,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.ofbiz.base.util.Debug; +import org.apache.ofbiz.entity.GenericValue; +import org.apache.ofbiz.security.SecurityUtil; /* * A Filter used to specify a whitelist of allowed paths to the OFBiz application. @@ -132,6 +134,11 @@ public class ControlFilter implements Filter { if (offset == -1) { offset = requestUri.length(); } + if (!GenericValue.getStackTraceAsString().contains("ControlFilterTests") + && SecurityUtil.containsFreemarkerInterpolation(httpRequest, httpResponse, requestUri)) { + return; + } + while (!allowedPaths.contains(requestUri.substring(0, offset))) { offset = requestUri.indexOf("/", offset + 1); if (offset == -1) {