Improved: Prevent URL parameters manipulation (OFBIZ-13147)

Handles Base64 encoded reverse shells
apache · Oct 23, 2024 · 8d83774 · 8d83774
1 parent fffa395
commit 8d83774
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 5 deletions.
diff --git 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
@@ -64,7 +64,7 @@ public void webShellTokensTesting() {
                 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 void webShellTokensTesting() {
             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
@@ -23,6 +23,7 @@
 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.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 void doFilter(HttpServletRequest req, HttpServletResponse resp, FilterCha
             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");
                 }