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

[Jacques Le Roux]

Hi,

This is an exceptional cross-post (dev and user) because I'd like to have the 
most possible attention.
So far I have most focused my effort to reject webshells on *nix OSs. Because I think that (almost?) nobody is deploying OFBiz on Windows servers in 
production.

I'd though have your opinion about that. The NSA document below gives a list of 
Windows executables that we could easily add.

What do you think?

TIA

Jacques

Le 15/02/2022 à 08:05, jler...@apache.org a écrit :
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 4fa50c8  Improved: Create a deny list to reject webshell tokens 
(OFBIZ-12324)
4fa50c8 is described below

commit 4fa50c87b708d0445b86af6d2062d336e543c971
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Tue Feb 15 07:54:17 2022 +0100

     Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
     
     After reviewing https://github.com/nsacyber/Mitigating-Web-Shells this mostly
     adds as tokens the "Linux applications commonly used by attackers and 
rarely
     launched by benign Apache applications"
---
  framework/security/config/security.properties            |  4 ++--
  .../java/org/apache/ofbiz/security/SecurityUtilTest.java | 16 +++++++++++++++-
  2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/framework/security/config/security.properties 
b/framework/security/config/security.properties
index ac75ee4..45bad62 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -252,8 +252,8 @@ allowAllUploads=
  c=freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@ 
page,\
                       
%eval,@eval,eval(,runtime,import,passthru,shell_exec,assert,str_rot13,system,decode,include,page
 ,\
                       chmod,mkdir,fopen,fclose,new 
file,upload,getfilename,download,getoutputstring,readfile,\
-                     python,perl ,/perl,ruby 
,/ruby,process,function,class,InputStream
-#-- IMPORTANT: when you change things here you need to do accordingly in 
SecurityUtilTest::webShellTokensTesting and run "gradlew test" --
+                     python,perl ,/perl,ruby 
,/ruby,process,function,class,InputStream,to_server,\
+                     
ifconfig,route,crontab,netstat,uname,hostname,iptables,whoami
  
  #-- 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/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
 
b/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
index 2659de1..2ffe1e6 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
@@ -62,7 +62,8 @@ public class SecurityUtilTest {
          // 
freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@ page,\
          // 
%eval,@eval,eval(,runtime,import,passthru,shell_exec,assert,str_rot13,system,decode,include,page
 ,\
          // chmod,mkdir,fopen,fclose,new 
file,upload,getfilename,download,getoutputstring,readfile,\
-        // python,perl ,/perl,ruby ,/ruby,process,function,class
+        // python,perl ,/perl,ruby 
,/ruby,process,function,class,InputStream,to_server,\
+        // ifconfig,route,crontab,netstat,uname,hostname,iptables,whoami
  
          try {
              List<String> allowed = new ArrayList<>();
@@ -113,6 +114,19 @@ public class SecurityUtilTest {
              assertFalse(SecuredUpload.isValidText("process", allowed));
              assertFalse(SecuredUpload.isValidText("function", allowed));
              assertFalse(SecuredUpload.isValidText("class", allowed));
+            assertFalse(SecuredUpload.isValidText("to_server", allowed));
+
+            assertFalse(SecuredUpload.isValidText("ifconfig", allowed));
+
+            assertFalse(SecuredUpload.isValidText("ifconfig", allowed));
+            assertFalse(SecuredUpload.isValidText("route", allowed));
+            assertFalse(SecuredUpload.isValidText("crontab", allowed));
+            assertFalse(SecuredUpload.isValidText("netstat", allowed));
+            assertFalse(SecuredUpload.isValidText("uname", allowed)); // found 
1 image (on 33 600) with this token in
+            assertFalse(SecuredUpload.isValidText("hostname", allowed));
+            assertFalse(SecuredUpload.isValidText("iptables", allowed));
+            assertFalse(SecuredUpload.isValidText("whoami", allowed));
+            // ip, ls, nc, ip, cat and pwd can'tbe used, too short
  
          } catch (IOException e) {
              fail(String.format("IOException occured : %s", e.getMessage()));