Fixed: [SECURITY] (CVE-2024-36104) Path traversal leading to RCE (OFB…

…IZ-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,
"&amp;" rather than "Y&amp;".
Also put all the privates methods used by doFilter just above it to clarify use.

framework/base/src/main/java/org/apache/ofbiz/base/util/StringUtil.java

@@ -21,6 +21,7 @@
 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 static List<String> split(String str, String delim) {
         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

framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java

@@ -143,8 +143,41 @@ private static boolean isSolrTest() {
         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 @@ && isSolrTest()) {
             }
             boolean bypass = true;
             if (queryString != null) {
-                bypass = isAnyAllowedToken(StringUtil.split(queryString.toLowerCase(), "Y&amp;"), ALLOWEDTOKENS);
+                List<String> queryStringList = StringUtil.splitWithStringSeparator(queryString.toLowerCase(), "&amp;");
+                bypass = isAnyAllowedToken(queryStringList, ALLOWEDTOKENS);
             }
             if (uriWithContext != null && !bypass) { // "null" allows tests with Mockito. ControlFilterTests sends null.
                 try {
@@ -236,38 +270,4 @@ && isSolrTest()) {
             }
         }
     }
-
-    /**
-     * 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;
-    }
 }