[ofbiz-framework] 02/03: Fixed: Secure the uploads (OFBIZ-12080)

[jleroux]

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

commit 835934a9c986f1381d24c8fd85478ed6ab908082
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Wed Feb 23 09:33:17 2022 +0100

    Fixed: Secure the uploads (OFBIZ-12080)
    
    Adds some tokens in security.properties::deniedWebShellTokens
    Updates SecurityUtilTest::webShellTokensTesting accordingly
---
 framework/security/config/security.properties                     | 4 ++--
 .../src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/framework/security/config/security.properties 
b/framework/security/config/security.properties
index 8ad620b..d41def2 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -249,10 +249,10 @@ allowAllUploads=
 #-- "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...
 #-- If you are sure you are safe for a token you can remove it, etc.
-deniedWebShellTokens=freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@
 page,<?php,exec(,\
+deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@
 page,<?php,exec(,\
                      
%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,to_server,wget ,\
+                     python,perl ,/perl,ruby 
,/ruby,process,function,class,InputStream,to_server,wget ,static,\
                      ifconfig,route,crontab,netstat,uname 
,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost
 
 
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 4cf8e37..0afa0f6 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
@@ -59,10 +59,10 @@ public class SecurityUtilTest {
     @Test
     public void webShellTokensTesting() {
         // Currently used
-        // 
freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@ 
page,<?php,exec(,\
+        // 
java.,beans,freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@
 page,<?php,exec(,\
         // 
%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,to_server,wget ,\
+        // python,perl ,/perl,ruby 
,/ruby,process,function,class,InputStream,to_server,wget ,static,\
         // 
ifconfig,route,crontab,netstat,uname,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost
 
         try {
@@ -72,6 +72,8 @@ public class SecurityUtilTest {
             allowed = new ArrayList<>();
             assertFalse(SecuredUpload.isValidText("hack.getFileName", 
allowed));
 
+            assertFalse(SecuredUpload.isValidText("java.", allowed));
+            assertFalse(SecuredUpload.isValidText("beans", allowed));
             assertFalse(SecuredUpload.isValidText("freemarker", allowed));
             assertFalse(SecuredUpload.isValidText("<script", allowed));
             assertFalse(SecuredUpload.isValidText("javascript", allowed));
@@ -117,7 +119,7 @@ public class SecurityUtilTest {
             assertFalse(SecuredUpload.isValidText("function", allowed));
             assertFalse(SecuredUpload.isValidText("class", allowed));
             assertFalse(SecuredUpload.isValidText("wget ", allowed));
-
+            assertFalse(SecuredUpload.isValidText("static", allowed));
 
             assertFalse(SecuredUpload.isValidText("ifconfig", allowed));
             assertFalse(SecuredUpload.isValidText("route", allowed));