[ofbiz-framework] branch release18.12 updated: Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)

[pgil]

This is an automated email from the ASF dual-hosted git repository.

pgil pushed a commit to branch release18.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/release18.12 by this push:
     new 523c25d  Improved: Create a deny list to reject webshell tokens 
(OFBIZ-12324)
523c25d is described below

commit 523c25d0ba87e57aebb1cbe9bab407a8fb4b42c5
Author: Gil Portenseigne <p...@apache.org>
AuthorDate: Wed Sep 22 15:03:30 2021 +0200

    Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
    
    Improve readability using stream api.
    Fix deniedWebShellTokens property where some blank space where removed
    and '// python' comment was inlined.
---
 framework/security/config/security.properties      |  4 +++-
 .../org/apache/ofbiz/security/SecuredUpload.java   | 26 +++-------------------
 2 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/framework/security/config/security.properties 
b/framework/security/config/security.properties
index 550fa15..5861ef4 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -205,7 +205,9 @@ allowAllUploads=
 #-- eg: 
https://www.acunetix.com/blog/articles/detection-prevention-introduction-web-shells-part-5/
 #-- "freemarker" should be OK, should not be used in Freemarker templates, not 
part of the syntax.
 #-- Else "template.utility.Execute" is a good replacement but not as much 
catching, who knows...
-deniedWebShellTokens=freemarker,import=\"java,runtime.getruntime().exec(,<%@page,<script,<body>,<form,php,javascript,%eval,@eval,importos//Python,passthru,exec,shell_exec,assert,str_rot13,system,phpinfo,base64_decode,chmod,mkdir,fopen,fclose,newfile,import,upload,getfilename,download,getoutputstring,readfile
+deniedWebShellTokens=freemarker,import=\"java,runtime.getruntime().exec(,<%@ 
page,<script,<body>,<form,php,\
+  javascript,%eval,@eval,import 
os,passthru,exec,shell_exec,assert,str_rot13,system,phpinfo,base64_decode,chmod,mkdir,\
+  fopen,fclose,new 
file,import,upload,getfilename,download,getoutputstring,readfile
 
 #-- uri used for login (cf jira OFBIZ-12047)
 #-- it's a list, each uri should be separated by comma, without space
diff --git 
a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java 
b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
index a5c7f50..afa2f7b 100644
--- 
a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
+++ 
b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
@@ -39,7 +39,6 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
@@ -622,12 +621,7 @@ public class SecuredUpload {
     }
 
     public static boolean isValidText(String content, List<String> allowed) 
throws IOException {
-        for (String token : DENIEDWEBSHELLTOKENS) {
-            if (!isValid(content, token, allowed)) {
-                return false;
-            }
-        }
-        return true;
+        return DENIEDWEBSHELLTOKENS.stream().allMatch(token -> 
isValid(content, token, allowed));
     }
 
     private static boolean isValid(String content, String string, List<String> 
allowed) {
@@ -643,26 +637,12 @@ public class SecuredUpload {
     }
 
     private static List<String> deniedFileExtensions() {
-        List<String> deniedFileExtensions = new LinkedList<>();
         String deniedExtensions = UtilProperties.getPropertyValue("security", 
"deniedFileExtensions");
-        if (UtilValidate.isNotEmpty(deniedExtensions)) {
-            List<String> deniedFileExtensionsList = 
StringUtil.split(deniedExtensions, ",");
-            for (String deniedExtension : deniedFileExtensionsList) {
-                deniedFileExtensions.add(deniedExtension);
-            }
-        }
-        return deniedFileExtensions;
+        return UtilValidate.isNotEmpty(deniedExtensions) ? 
StringUtil.split(deniedExtensions, ",") : new ArrayList<>();
     }
 
     private static List<String> deniedWebShellTokens() {
-        List<String> deniedWebShellTokens = new LinkedList<>();
         String deniedTokens = UtilProperties.getPropertyValue("security", 
"deniedWebShellTokens");
-        if (UtilValidate.isNotEmpty(deniedTokens)) {
-            List<String> deniedWebShellTokensList = 
StringUtil.split(deniedTokens, ",");
-            for (String deniedToken : deniedWebShellTokensList) {
-                deniedWebShellTokens.add(deniedToken);
-            }
-        }
-        return deniedWebShellTokens;
+        return UtilValidate.isNotEmpty(deniedTokens) ? 
StringUtil.split(deniedTokens, ",") : new ArrayList<>();
     }
 }