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 06:50:57 UTC
[ofbiz-framework] 02/02: 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 trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
commit 2aeb282cdc792e4a30274bffb57d63f3829bcca7
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 03c6804e89..f546a4c595 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);
}
}
}