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

@@ -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

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

@@ -22,7 +22,6 @@
 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.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;
@@ -160,21 +159,6 @@ private static List<String> getAllowedTokens() {
         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
@@ -198,7 +182,7 @@ public void doFilter(HttpServletRequest req, HttpServletResponse resp, FilterCha
         } 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");
@@ -227,12 +211,7 @@ && isSolrTest()) {
                     throw new RuntimeException("For security reason this URL is not accepted");
                 }
             }
-            boolean bypass = true;
-            if (queryString != null) {
-                List<String> queryStringList = StringUtil.splitWithStringSeparator(queryString.toLowerCase(), "&amp;");
-                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()