(ofbiz-framework) branch trunk updated: Improved: Prevent URL parameters manipulation (OFBIZ-13147)

[jleroux]

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 42b9ad8dbd Improved: Prevent URL parameters manipulation (OFBIZ-13147)
42b9ad8dbd is described below

commit 42b9ad8dbd416bf7ed73ad95e94681329cc83ac7
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Thu Oct 24 20:35:39 2024 +0200

    Improved: Prevent URL parameters manipulation (OFBIZ-13147)
    
    The "JavaScriptEnabled=Y" and "&wt=javabin" references are weaknesses.
    I temporarily put them in ControlFilter::doFilter to allow things (demo and
    integration tests) to work for my test (only possible on a site w. domain 
IP),
    ie not locally.
    
    I think we can remove  "JavaScriptEnabled=Y". I put it there because we use 
it
    in links at https://ofbiz.apache.org/ofbiz-demos.html. Maybe other places 
where
    it's easy to remove w/o side effects. It's anyway an user preference, not
    mandatory in query string.
    
    I needed "&wt=javabin" for the Solr tests to pass. Sometimes ago I already 
faced
    a such issue. And then put in place what's needed. 
ControlFilter::isSolrTest is
    the solution by generalising this usage.
---
 .../java/org/apache/ofbiz/webapp/control/ControlFilter.java  | 12 ++++++------
 1 file changed, 6 insertions(+), 6 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 7a7511271f..a4c0e59400 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
@@ -131,6 +131,10 @@ public class ControlFilter extends HttpFilter {
                                : 
Arrays.stream(paths.split(":")).collect(Collectors.toSet());
     }
 
+    private static boolean isSolrTest() {
+        return 
!GenericValue.getStackTraceAsString().contains("ControlFilterTests")
+                && null == System.getProperty("SolrDispatchFilter");
+    }
     /**
      * Makes allowed paths pass through while redirecting the others to a fix 
location.
      */
@@ -159,9 +163,7 @@ public class ControlFilter extends HttpFilter {
 
             GenericValue userLogin = (GenericValue) 
session.getAttribute("userLogin");
             if (!LoginWorker.hasBasePermission(userLogin, req)) { // Allows 
UEL and FlexibleString (OFBIZ-12602)
-                if 
(!GenericValue.getStackTraceAsString().contains("ControlFilterTests")
-                        && null == System.getProperty("SolrDispatchFilter") // 
Allows Solr tests
-                        && SecurityUtil.containsFreemarkerInterpolation(req, 
resp, uri)) {
+                if (isSolrTest() && 
SecurityUtil.containsFreemarkerInterpolation(req, resp, uri)) {
                     return;
                 }
             }
@@ -170,11 +172,9 @@ public class ControlFilter extends HttpFilter {
             String queryString = req.getQueryString();
             if (queryString != null) {
                 queryString = URLDecoder.decode(queryString, "UTF-8");
-                // wt=javabin allows Solr tests, see 
https://cwiki.apache.org/confluence/display/solr/javabin
                 if (UtilValidate.isUrlInString(queryString)
                         || !SecuredUpload.isValidText(queryString, 
Collections.emptyList())
-                        && !(queryString.contains("JavaScriptEnabled=Y")
-                                || queryString.contains("wt=javabin"))) {
+                        && isSolrTest()) {
                     Debug.logError("For security reason this URL is not 
accepted", MODULE);
                     throw new RuntimeException("For security reason this URL 
is not accepted");
                 }