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 976b63b Improved: Create a deny list to reject webshell tokens
(OFBIZ-12324)
976b63b is described below
commit 976b63bb5b49ed35f2c7d7834a53268e7859a719
Author: Jacques Le Roux <jacques.le.r...@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 edc5d4c..08433a4 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -232,6 +232,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
#-- Popup last-visited time from database after user has logged in.
#-- So users can know of any unauthorised access to their accounts.
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 7c7f462..6741975 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
@@ -615,42 +616,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) {
@@ -676,4 +647,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;
+ }
}
