You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2021/09/20 16:16:49 UTC

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

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

jleroux 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 8b852ff  Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
8b852ff is described below

commit 8b852ff6120decedd0d0fa197a1810ae0aa1b591
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Mon Sep 20 18:13:47 2021 +0200

    Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
    
    Extracts the list of words used in SecuredUpload::isValidText in a
    deniedWebShellTokens property in security.properties
---
 framework/security/config/security.properties      |  7 +++
 .../org/apache/ofbiz/security/SecuredUpload.java   | 55 ++++++++--------------
 2 files changed, 26 insertions(+), 36 deletions(-)

diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index bd45b9f..550fa15 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -199,6 +199,13 @@ deniedFileExtensions=html,htm,php,php2,hph3,php4,php5,asp,aspx,ascx,jsp,jspx,cfm
 #-- people may like to allow more than what is allowed OOTB
 #-- As it name says, allowAllUploads opens all possibilities
 allowAllUploads=
+#--
+#-- List of denied tokens often part of webshells
+#-- TODO.... to be continued with known webshell contents... a complete allow list is impossible anyway...
+#-- 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
 
 #-- 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 4dc79d7..a5c7f50 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
@@ -98,6 +98,7 @@ public class SecuredUpload {
 
     private static final String MODULE = SecuredUpload.class.getName();
     private static final List<String> DENIEDFILEEXTENSIONS = deniedFileExtensions();
+    private static final List<String> DENIEDWEBSHELLTOKENS = deniedWebShellTokens();
 
     /**
      * @param fileToCheck
@@ -621,42 +622,12 @@ public class SecuredUpload {
     }
 
     public static boolean isValidText(String content, List<String> allowed) throws IOException {
-        // "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...
-        return isValid(content, "freemarker", allowed)
-                && isValid(content, "freemarker", allowed)
-                && isValid(content, "import=\"java", allowed)
-                && isValid(content, "runtime.getruntime().exec(", allowed)
-                && isValid(content, "<%@ page", allowed)
-                && isValid(content, "<script", allowed)
-                && isValid(content, "<body>", allowed)
-                && isValid(content, "<form", allowed)
-                && isValid(content, "php", allowed)
-                && isValid(content, "javascript", allowed)
-                && isValid(content, "%eval", allowed)
-                && isValid(content, "@eval", allowed)
-                && isValid(content, "import os", allowed) // Python
-                && isValid(content, "passthru", allowed)
-                && isValid(content, "exec", allowed)
-                && isValid(content, "shell_exec", allowed)
-                && isValid(content, "assert", allowed)
-                && isValid(content, "str_rot13", allowed)
-                && isValid(content, "system", allowed)
-                && isValid(content, "phpinfo", allowed)
-                && isValid(content, "base64_decode", allowed)
-                && isValid(content, "chmod", allowed)
-                && isValid(content, "mkdir", allowed)
-                && isValid(content, "fopen", allowed)
-                && isValid(content, "fclose", allowed)
-                && isValid(content, "new file", allowed)
-                && isValid(content, "import", allowed)
-                && isValid(content, "upload", allowed)
-                && isValid(content, "getfilename", allowed)
-                && isValid(content, "download", allowed)
-                && isValid(content, "getoutputstring", allowed)
-                && isValid(content, "readfile", allowed);
-        // TODO.... to be continued with known webshell contents... a complete allow list is impossible anyway...
-        // eg: https://www.acunetix.com/blog/articles/detection-prevention-introduction-web-shells-part-5/
+        for (String token : DENIEDWEBSHELLTOKENS) {
+            if (!isValid(content, token, allowed)) {
+                return false;
+            }
+        }
+        return true;
     }
 
     private static boolean isValid(String content, String string, List<String> allowed) {
@@ -682,4 +653,16 @@ public class SecuredUpload {
         }
         return deniedFileExtensions;
     }
+
+    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;
+    }
 }