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 2025/01/16 15:05:44 UTC
(ofbiz-framework) branch trunk updated (36b1e6d409 -> f95ab015f3)
This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
from 36b1e6d409 Publish build scans to develocity.apache.org #5165 (#871)
new 449fd10cc9 Revert "Improved: Path traversal leading to RCE (OFBIZ-13092)"
new f95ab015f3 Fixed: [SECURITY] (CVE-2024-36104) Path traversal leading to RCE (OFBIZ-13092)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
framework/security/config/security.properties | 2 +-
.../org/apache/ofbiz/security/SecuredUpload.java | 6 ---
.../apache/ofbiz/webapp/control/ControlFilter.java | 55 +++++++++++++++++++---
3 files changed, 49 insertions(+), 14 deletions(-)
(ofbiz-framework) 01/02: Revert "Improved: Path traversal leading to RCE (OFBIZ-13092)"
Posted by jl...@apache.org.
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 449fd10cc906ec738a631b0ae35dbb449a7d3f39
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Tue Jan 14 17:30:27 2025 +0100
Revert "Improved: Path traversal leading to RCE (OFBIZ-13092)"
This reverts commit 5f8bb71aae9b65f031478784374bab4326e57f1f.
---
.../main/java/org/apache/ofbiz/webapp/control/ControlFilter.java | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
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 a8e3301ed7..ca8d2cca69 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
@@ -174,19 +174,20 @@ public class ControlFilter extends HttpFilter {
queryString = URLDecoder.decode(queryString, "UTF-8");
if (UtilValidate.isUrlInString(queryString)
|| !SecuredUpload.isValidText(queryString.toLowerCase(), SecuredUpload.getallowedTokens(), true)
- && isSolrTest()) {
+ && isSolrTest()) {
Debug.logError("For security reason this URL is not accepted", MODULE);
throw new RuntimeException("For security reason this URL is not accepted");
}
}
- if (uriWithContext != null) { // Allow tests with Mockito. ControlFilterTests send null
+ String initialURI = req.getRequestURI();
+ if (initialURI != null) { // Allow tests with Mockito. ControlFilterTests send null
try {
- String uRIFiltered = new URI(uriWithContext)
+ String uRIFiltered = new URI(initialURI)
.normalize().toString()
.replaceAll(";", "")
.replaceAll("(?i)%2e", "");
- if (!uriWithContext.equals(uRIFiltered)) {
+ if (!initialURI.equals(uRIFiltered)) {
Debug.logError("For security reason this URL is not accepted", MODULE);
throw new RuntimeException("For security reason this URL is not accepted");
}
(ofbiz-framework) 02/02: Fixed: [SECURITY] (CVE-2024-36104) Path traversal leading to RCE (OFBIZ-13092)
Posted by jl...@apache.org.
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 f95ab015f353ca820b3718747575898e87dd9084
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Thu Jan 16 11:28:00 2025 +0100
Fixed: [SECURITY] (CVE-2024-36104) Path traversal leading to RCE (OFBIZ-13092)
I've got a strange issue that seems caused by one of this Jira modification:
On demo environment, when clicking on several sort field links, a
'For security reason this URL is not accepted' error is thrown.
For instance, if you perform a search on find party screen and try to sort by
partyId, createdDate or else, you will get this message.
I tried to reproduce it in a local trunk environment without success : the
uRIFiltered and the initialURI are the same.
jleroux: but indeed when the same is done on a domain site, for instance
official OFBiz demos, it's reproducible in all demo instances.
This fixes it, with some more: refactoring, formatting and renaming
Thanks: Leïla for report and details.
---
framework/security/config/security.properties | 2 +-
.../org/apache/ofbiz/security/SecuredUpload.java | 6 ---
.../apache/ofbiz/webapp/control/ControlFilter.java | 62 ++++++++++++++++++----
3 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index 6fbc9b9ea8..141ec7e899 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
+allowedTokens=$SHA$OFBiz$2naHrANKTniFcgLJk4oXr3IRQ48,$SHA$OFBiz$evAu1vcT5d1tjVXFTeVXU-6aNz8,$SHA$OFBiz$-MaMN-Dui294v86UT1T8BkG3v8k
#-- 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/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
index 8c88d46278..cfe6e1f9ee 100644
--- a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
+++ b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
@@ -910,12 +910,6 @@ public class SecuredUpload {
return UtilValidate.isNotEmpty(deniedTokens) ? StringUtil.split(deniedTokens, ",") : new ArrayList<>();
}
- public static List<String> getallowedTokens() {
- String allowedTokens = UtilProperties.getPropertyValue("security", "allowedTokens");
- return UtilValidate.isNotEmpty(allowedTokens) ? StringUtil.split(allowedTokens, ",") : new ArrayList<>();
- }
-
-
private static boolean checkMaxLinesLength(String fileToCheck) {
if (MAXLINELENGTH == 0) {
return true;
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 ca8d2cca69..2c97b0c9f2 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,8 +22,11 @@ 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;
+import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
@@ -38,7 +41,10 @@ import javax.servlet.http.HttpSession;
import org.apache.commons.lang.BooleanUtils;
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;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.security.SecuredFreemarker;
@@ -85,6 +91,8 @@ public class ControlFilter extends HttpFilter {
private int errorCode;
/** The list of all path prefixes that are allowed. */
private Set<String> allowedPaths;
+ private static final List<String> ALLOWEDTOKENS = getAllowedTokens();
+
@Override
public void init(FilterConfig conf) throws ServletException {
@@ -139,8 +147,7 @@ public class ControlFilter extends HttpFilter {
* Makes allowed paths pass through while redirecting the others to a fix location.
*/
@Override
- public void doFilter(HttpServletRequest req, HttpServletResponse resp, FilterChain chain)
- throws IOException, ServletException {
+ public void doFilter(HttpServletRequest req, HttpServletResponse resp, FilterChain chain) throws IOException, ServletException {
String context = req.getContextPath();
HttpSession session = req.getSession();
@@ -158,7 +165,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 = req.getRequestURI();
+ String uriWithContext = URLDecoder.decode(req.getRequestURI(), "UTF-8");
String uri = uriWithContext.substring(context.length());
GenericValue userLogin = (GenericValue) session.getAttribute("userLogin");
@@ -169,25 +176,35 @@ public class ControlFilter extends HttpFilter {
}
// Reject wrong URLs
- String queryString = req.getQueryString();
+ String queryString = null;
+ try {
+ queryString = new URI(uriWithContext).getQuery();
+
+ } catch (URISyntaxException e) {
+ Debug.logError("Weird URI: " + e, MODULE);
+ throw new RuntimeException(e);
+ }
+
if (queryString != null) {
queryString = URLDecoder.decode(queryString, "UTF-8");
if (UtilValidate.isUrlInString(queryString)
- || !SecuredUpload.isValidText(queryString.toLowerCase(), SecuredUpload.getallowedTokens(), true)
- && isSolrTest()) {
+ || !SecuredUpload.isValidText(queryString.toLowerCase(), ALLOWEDTOKENS, true)
+ && isSolrTest()) {
Debug.logError("For security reason this URL is not accepted", MODULE);
throw new RuntimeException("For security reason this URL is not accepted");
}
}
-
- String initialURI = req.getRequestURI();
- if (initialURI != null) { // Allow tests with Mockito. ControlFilterTests send null
+ boolean bypass = true;
+ if (queryString != null) {
+ bypass = isAnyAllowedToken(StringUtil.split(queryString.toLowerCase(), "Y&"), ALLOWEDTOKENS);
+ }
+ if (uriWithContext != null && !bypass) { // "null" allows tests with Mockito. ControlFilterTests sends null.
try {
- String uRIFiltered = new URI(initialURI)
+ String uRIFiltered = new URI(uriWithContext)
.normalize().toString()
.replaceAll(";", "")
.replaceAll("(?i)%2e", "");
- if (!initialURI.equals(uRIFiltered)) {
+ if (!uriWithContext.equals(uRIFiltered)) {
Debug.logError("For security reason this URL is not accepted", MODULE);
throw new RuntimeException("For security reason this URL is not accepted");
}
@@ -230,4 +247,27 @@ public class ControlFilter extends HttpFilter {
private void redirect(HttpServletResponse resp, String contextPath) throws IOException {
resp.sendRedirect(redirectPathIsUrl ? redirectPath : (contextPath + redirectPath));
}
+
+ private static List<String> getAllowedTokens() {
+ String allowedTokens = UtilProperties.getPropertyValue("security", "allowedTokens");
+ 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;
+ }
}