This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch release24.09
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release24.09 by this push:
new 99de4fa7fe Improved: [SECURITY] (CVE-2024-36104) Path traversal
leading to RCE (OFBIZ-13092)
99de4fa7fe is described below
commit 99de4fa7fe55e078627fad079fc6b481d453e7b3
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Sat Jan 18 10:11:14 2025 +0100
Improved: [SECURITY] (CVE-2024-36104) Path traversal leading to RCE
(OFBIZ-13092)
Using allowedTokens was a bad idea (when all you have is a (new!) hammer,
everything looks like a nail).
It came to my mind that I already used StringEscapeUtils::unescapeHtml4
in several places and in this case it's the tool of choice.
Actually I thought about it when beginning, but was unsure it would be
enough.
Now it's clear in my mind and this replaces allowedTokens by simply
StringEscapeUtils::unescapeHtml4
So this also removes the recently added allowedTokens in
security.properties.
They are now useless.
Also it's quite better because it handles all cases known or unknown.
Better keep allowedTokens list as short as possible.
---
framework/security/config/security.properties | 2 +-
.../apache/ofbiz/webapp/control/ControlFilter.java | 27 +++-------------------
2 files changed, 4 insertions(+), 25 deletions(-)
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index 38b0cf0beb..544dc16421 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -288,7 +288,7 @@
deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,body ,<form
#-- SHA-1 versions of tokens containing (as String) at least one
deniedWebShellTokens
#-- This is notably used to allow special values in query parameters.
#-- If you add a token beware that it does not content ",". It's the separator.
-allowedTokens=$SHA$OFBiz$2naHrANKTniFcgLJk4oXr3IRQ48,$SHA$OFBiz$evAu1vcT5d1tjVXFTeVXU-6aNz8,$SHA$OFBiz$-MaMN-Dui294v86UT1T8BkG3v8k
+allowedTokens=$SHA$OFBiz$2naHrANKTniFcgLJk4oXr3IRQ48
#-- RegExp to secure groovy script execution. If the regExp match a script, it
would be disabled and OFBiz run nothing.
#-- In this case, you will have on log the original script with it hash. The
hash can be added on allowedScriptletHashes
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 2fb2439501..4ccd968f54 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
@@ -22,7 +22,6 @@ import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -39,9 +38,9 @@ import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import org.apache.commons.lang.BooleanUtils;
+import org.apache.commons.text.StringEscapeUtils;
import org.apache.commons.validator.routines.UrlValidator;
import org.apache.logging.log4j.ThreadContext;
-import org.apache.ofbiz.base.crypto.HashCrypt;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.StringUtil;
import org.apache.ofbiz.base.util.UtilProperties;
@@ -157,21 +156,6 @@ public class ControlFilter extends HttpFilter {
return UtilValidate.isNotEmpty(allowedTokens) ?
StringUtil.split(allowedTokens, ",") : new ArrayList<>();
}
- // Check there is any allowedToken in URL
- private static boolean isAnyAllowedToken(List<String> queryParameters,
List<String> allowed) {
- boolean isOK = false;
- for (String parameter : queryParameters) {
- parameter = parameter.substring(0, parameter.indexOf("=") + 1);
- if (allowed.contains(HashCrypt.cryptBytes("SHA", "OFBiz",
parameter.getBytes(StandardCharsets.UTF_8)))) {
- isOK = true;
- break;
- } else {
- continue;
- }
- }
- return isOK;
- }
-
/**
* Makes allowed paths pass through while redirecting the others to a fix
location.
* Reject wrong URLs
@@ -195,7 +179,7 @@ public class ControlFilter extends HttpFilter {
} else if (req.getAttribute(FORWARDED_FROM_SERVLET) == null
&& !allowedPaths.isEmpty()) {
// Get the request URI without the webapp mount point.
- String uriWithContext = URLDecoder.decode(req.getRequestURI(),
"UTF-8");
+ String uriWithContext =
StringEscapeUtils.unescapeHtml4(URLDecoder.decode(req.getRequestURI(),
"UTF-8"));
String uri = uriWithContext.substring(context.length());
GenericValue userLogin = (GenericValue)
session.getAttribute("userLogin");
@@ -224,12 +208,7 @@ public class ControlFilter extends HttpFilter {
throw new RuntimeException("For security reason this URL
is not accepted");
}
}
- boolean bypass = true;
- if (queryString != null) {
- List<String> queryStringList =
StringUtil.splitWithStringSeparator(queryString.toLowerCase(), "&");
- bypass = isAnyAllowedToken(queryStringList, ALLOWEDTOKENS);
- }
- if (uriWithContext != null && !bypass) { // "null" allows tests
with Mockito. ControlFilterTests sends null.
+ if (uriWithContext != null) { // "null" allows tests with Mockito
because ControlFilterTests sends null.
try {
String uRIFiltered = new URI(uriWithContext)
.normalize().toString()
