You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2022/04/04 07:17:32 UTC
[ofbiz-framework] branch release22.01 updated: Improved: Prevent Freemarker interpolation in fields (OFBIZ-12594)
This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch release22.01
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release22.01 by this push:
new 3284ad3994 Improved: Prevent Freemarker interpolation in fields (OFBIZ-12594)
3284ad3994 is described below
commit 3284ad3994331eb322966d1488ed37c8e1b52220
Author: Jacques Le Roux <ja...@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
---
.../ofbiz/product/category/SeoContextFilter.java | 7 +-
framework/security/config/security.properties | 4 ++
.../org/apache/ofbiz/security/SecurityUtil.java | 78 +++++++++++++++++++++-
.../apache/ofbiz/webapp/control/ControlFilter.java | 9 ++-
4 files changed, 94 insertions(+), 4 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 bf376cf863..eee52f8e99 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,11 +48,12 @@ 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.SeoConfigUtil;
import org.apache.ofbiz.webapp.control.ConfigXMLReader;
import org.apache.ofbiz.webapp.control.ConfigXMLReader.ControllerConfig;
import org.apache.ofbiz.webapp.control.ControlFilter;
import org.apache.ofbiz.webapp.control.WebAppConfigurationException;
-import org.apache.ofbiz.webapp.SeoConfigUtil;
import org.apache.oro.text.regex.Pattern;
import org.apache.oro.text.regex.Perl5Matcher;
@@ -115,6 +116,10 @@ public final class SeoContextFilter implements Filter {
uri = uri + "?" + queryString;
}
+ if (SecurityUtil.containsFreemarkerInterpolation(httpRequest, httpResponse, uri)) {
+ return;
+ }
+
boolean forwarded = forwardUri(httpResponse, uri);
if (forwarded) {
return;
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index f106f9b21f..2c60cdcda3 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -281,3 +281,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 6eabff180c..8bd470e987 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;
@@ -55,7 +65,7 @@ public final class SecurityUtil {
* Return true if given userLogin possess at least one of the adminPermission
* @param delegator
* @param userLoginId
- * @return
+ * @return boolean
*/
public static boolean hasUserLoginAdminPermission(Delegator delegator, String userLoginId) {
if (UtilValidate.isEmpty(userLoginId)) return false;
@@ -78,7 +88,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<>();
@@ -161,4 +171,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 bf9551f616..8c76628225 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
@@ -36,6 +36,8 @@ import org.apache.commons.lang.BooleanUtils;
import org.apache.commons.validator.routines.UrlValidator;
import org.apache.logging.log4j.ThreadContext;
import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.entity.GenericValue;
+import org.apache.ofbiz.security.SecurityUtil;
/**
* A Filter used to specify an allowlist of allowed paths to the OFBiz application.
@@ -147,6 +149,11 @@ public class ControlFilter extends HttpFilter {
String uriWithContext = req.getRequestURI();
String uri = uriWithContext.substring(context.length());
+ if (!GenericValue.getStackTraceAsString().contains("ControlFilterTests")
+ && SecurityUtil.containsFreemarkerInterpolation(req, resp, uri)) {
+ return;
+ }
+
// Check if the requested URI is allowed.
if (allowedPaths.stream().anyMatch(uri::startsWith)) {
try {
@@ -165,7 +172,7 @@ public class ControlFilter extends HttpFilter {
}
if (Debug.infoOn()) {
Debug.logInfo("[Filtered request]: " + uriWithContext + " --> "
- + (redirectPath == null ? errorCode : redirectPath), MODULE);
+ + (redirectPath == null ? errorCode : redirectPath), MODULE);
}
}
}