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
The following commit(s) were added to refs/heads/trunk by this push: new 70d8415f86 Fixed: [SECURITY] (CVE-2024-36104) Path traversal leading to RCE (OFBIZ-13092) 70d8415f86 is described below commit 70d8415f86ad7a89359fbd20117553aae513cb4d Author: Jacques Le Roux <jacques.le.r...@les7arts.com> AuthorDate: Fri Jan 17 10:59:28 2025 +0100 Fixed: [SECURITY] (CVE-2024-36104) Path traversal leading to RCE (OFBIZ-13092) Adds a StringUtil::splitWithStringSeparator. I crossed issue using StringUtil::split it's said that <<delim the delimiter character(s)>> with a (s) But it does not work as expected with several character(s). In ControlFilter::doFilter uses splitWithStringSeparator instead of split. Uses decoded requestUri everywhere, and to split query string, though it worked, "&" rather than "Y&". Also put all the privates methods used by doFilter just above it to clarify use. --- .../org/apache/ofbiz/base/util/StringUtil.java | 15 +++++ .../apache/ofbiz/webapp/control/ControlFilter.java | 70 +++++++++++----------- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/StringUtil.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/StringUtil.java index 68fa424224..1652e684ea 100644 --- a/framework/base/src/main/java/org/apache/ofbiz/base/util/StringUtil.java +++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/StringUtil.java @@ -21,6 +21,7 @@ package org.apache.ofbiz.base.util; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.net.URLEncoder; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; @@ -141,6 +142,20 @@ public final class StringUtil { return splitList; } + /** + * Splits a String on a String Separator into a List of Strings. + * @param str the String to split + * @param separator the String Separator to split the str String + * @return a list of Strings or null if one of the parameters is null + */ + public static List<String> splitWithStringSeparator(String str, String separator) { + if (str == null || separator == null) { + return null; + } + + return Arrays.asList(str.split(separator)); + } + /** * Creates a Map from an encoded name/value pair string * @param str The string to decode and format 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 2c97b0c9f2..ae215c83bc 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 @@ -143,8 +143,41 @@ public class ControlFilter extends HttpFilter { return !GenericValue.getStackTraceAsString().contains("ControlFilterTests") && null == System.getProperty("SolrDispatchFilter"); } + + /** + * Sends an HTTP response redirecting to {@code redirectPath}. + * @param resp The response to send + * @param contextPath the prefix to add to the redirection when + * {@code redirectPath} is a relative URI. + * @throws IOException when redirection has not been properly sent. + */ + 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; + } + /** * Makes allowed paths pass through while redirecting the others to a fix location. + * Reject wrong URLs */ @Override public void doFilter(HttpServletRequest req, HttpServletResponse resp, FilterChain chain) throws IOException, ServletException { @@ -196,7 +229,8 @@ public class ControlFilter extends HttpFilter { } boolean bypass = true; if (queryString != null) { - bypass = isAnyAllowedToken(StringUtil.split(queryString.toLowerCase(), "Y&"), ALLOWEDTOKENS); + List<String> queryStringList = StringUtil.splitWithStringSeparator(queryString.toLowerCase(), "&"); + bypass = isAnyAllowedToken(queryStringList, ALLOWEDTOKENS); } if (uriWithContext != null && !bypass) { // "null" allows tests with Mockito. ControlFilterTests sends null. try { @@ -236,38 +270,4 @@ public class ControlFilter extends HttpFilter { } } } - - /** - * Sends an HTTP response redirecting to {@code redirectPath}. - * @param resp The response to send - * @param contextPath the prefix to add to the redirection when - * {@code redirectPath} is a relative URI. - * @throws IOException when redirection has not been properly sent. - */ - 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; - } }