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) {
