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

commit 8d837744413d8b3f1c2c28ff3a437103d0c7a8f5
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Wed Oct 23 09:16:04 2024 +0200

    Improved: Prevent URL parameters manipulation (OFBIZ-13147)
    
    Handles Base64 encoded reverse shells
---
 framework/security/config/security.properties                     | 2 +-
 .../src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java | 6 +++---
 .../main/java/org/apache/ofbiz/webapp/control/ControlFilter.java  | 8 +++++++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/framework/security/config/security.properties 
b/framework/security/config/security.properties
index 2e8a42c420..2c54c8a454 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -278,7 +278,7 @@ 
deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,body ,<form
                      chmod,mkdir,fopen,fclose,new 
file,upload,getfilename,download,getoutputstring,readfile,iframe,object,embed,onload,build,\
                      python,perl ,/perl,ruby 
,/ruby,process,function,class,InputStream,to_server,wget 
,static,assign,webappPath,\
                      ifconfig,route,crontab,netstat,uname 
,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost,thread,require,gzdeflate,\
-                     execute,println,calc,calculate,touch,curl
+                     execute,println,calc,touch,curl,base64
 
 allowStringConcatenationInUploadedFiles=false
 
diff --git 
a/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
 
b/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
index 6f46591fc6..93bcbf8a12 100644
--- 
a/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
+++ 
b/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
@@ -64,7 +64,7 @@ public class SecurityUtilTest {
                 chmod,mkdir,fopen,fclose,new 
file,upload,getfilename,download,getoutputstring,readfile,iframe,object,embed,onload,build,\
                 python,perl ,/perl,ruby 
,/ruby,process,function,class,InputStream,to_server,wget 
,static,assign,webappPath,\
                 ifconfig,route,crontab,netstat,uname 
,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost,thread,require,gzdeflate,\
-                execute,println,calc,calculate,touch,curl
+                execute,println,calc,touch,curl,base64
          */
         try {
             List<String> allowed = new ArrayList<>();
@@ -150,9 +150,9 @@ public class SecurityUtilTest {
             assertFalse(SecuredUpload.isValidText("execute", allowed));
             assertFalse(SecuredUpload.isValidText("println", allowed));
             assertFalse(SecuredUpload.isValidText("calc", allowed));
-            assertFalse(SecuredUpload.isValidText("calculate", allowed));
-            assertFalse(SecuredUpload.isValidText("curl", allowed));
             assertFalse(SecuredUpload.isValidText("touch", allowed));
+            assertFalse(SecuredUpload.isValidText("curl", allowed));
+            assertFalse(SecuredUpload.isValidText("base64", allowed));
         } catch (IOException e) {
             fail(String.format("IOException occured : %s", e.getMessage()));
         }
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 9aa1734515..97fc721cc1 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
@@ -23,6 +23,7 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URLDecoder;
 import java.util.Arrays;
+import java.util.Base64;
 import java.util.Collections;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -41,6 +42,7 @@ import org.apache.logging.log4j.ThreadContext;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.UtilValidate;
 import org.apache.ofbiz.entity.GenericValue;
+import org.apache.ofbiz.security.SecuredUpload;
 import org.apache.ofbiz.security.SecurityUtil;
 
 
@@ -169,7 +171,11 @@ public class ControlFilter extends HttpFilter {
             String queryString = req.getQueryString();
             if (queryString != null) {
                 queryString = URLDecoder.decode(queryString, "UTF-8");
-                if (UtilValidate.isUrl(queryString)) {
+                if (UtilValidate.isUrl(queryString)
+                        || !SecuredUpload.isValidText(queryString, 
Collections.emptyList())
+                        || 
!SecuredUpload.isValidText(Base64.getDecoder().decode(queryString).toString(), 
Collections.emptyList())
+                        || 
!SecuredUpload.isValidText(Base64.getMimeDecoder().decode(queryString).toString(),
 Collections.emptyList())
+                        || 
!SecuredUpload.isValidText(Base64.getUrlDecoder().decode(queryString).toString(),
 Collections.emptyList())) { // ...
                     Debug.logError("For security reason this URL is not 
accepted", MODULE);
                     throw new RuntimeException("For security reason this URL 
is not accepted");
                 }