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 2022/02/14 13:31:26 UTC
[ofbiz-framework] branch release22.01 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 release22.01
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release22.01 by this push:
new 183b507 Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
183b507 is described below
commit 183b507041aa5d729f7e3f287a3aed8c0bfbdf4f
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Mon Feb 14 13:16:55 2022 +0100
Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
After reviewing https://docs.oracle.com/cd/B14099_19/web.1012/b14014/jspxml.htm,
since we use "<jsp:" this removes "scriptlet>", "declaration>" and "expression>".
They are redundant with "<jsp:"; "<jsp:" covers those already.
Also adds eval( for js and shrinks base64_decode to decode, and processbuilder
to process because of OFBIZ 12571
I always test the denied tokens against a lib of images with 33 600 elements (
3.99 GB). If an image contains the token it can't be in the denied list.
---
framework/security/config/security.properties | 6 +++---
.../org/apache/ofbiz/security/SecurityUtilTest.java | 19 ++++++++-----------
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index 3c6ae64..ac75ee4 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:,scriptlet>,declaration>,expression>,<c:out,taglib,<prefix,<%@ page,\
- %eval,@eval,runtime,import,passthru,shell_exec,assert,str_rot13,system,base64_decode,include,\
+deniedWebShellTokens=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,processbuilder,function,class
+ 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" --
#-- Popup last-visited time from database after user has logged in.
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 9757733..2659de1 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:,scriptlet>,declaration>,expression>,<c:out,taglib,<prefix,<%@ page
- // %eval,@eval,runtime,import,passthru,shell_exec,assert,str_rot13,system,base64_decode,include
- // chmod,mkdir,fopen,fclose,new file,upload,getfilename,download,getoutputstring,readfile
- // python,perl ,/perl,ruby ,/ruby,processbuilder,function,class
+ // 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
try {
List<String> allowed = new ArrayList<>();
@@ -77,9 +77,6 @@ public class SecurityUtilTest {
assertFalse(SecuredUpload.isValidText("<body", allowed));
assertFalse(SecuredUpload.isValidText("<form", allowed));
assertFalse(SecuredUpload.isValidText("<jsp:", allowed));
- assertFalse(SecuredUpload.isValidText("scriptlet>", allowed));
- assertFalse(SecuredUpload.isValidText("declaration>", allowed));
- assertFalse(SecuredUpload.isValidText("expression>", allowed));
assertFalse(SecuredUpload.isValidText("<c:out", allowed));
assertFalse(SecuredUpload.isValidText("taglib", allowed));
assertFalse(SecuredUpload.isValidText("<prefix", allowed));
@@ -94,7 +91,7 @@ public class SecurityUtilTest {
assertFalse(SecuredUpload.isValidText("assert", allowed));
assertFalse(SecuredUpload.isValidText("str_rot13", allowed));
assertFalse(SecuredUpload.isValidText("system", allowed));
- assertFalse(SecuredUpload.isValidText("base64_decode", allowed));
+ assertFalse(SecuredUpload.isValidText("decode", allowed));
assertFalse(SecuredUpload.isValidText("include", allowed));
assertFalse(SecuredUpload.isValidText("chmod", allowed));
@@ -113,9 +110,9 @@ public class SecurityUtilTest {
assertFalse(SecuredUpload.isValidText("/perl", allowed));
assertFalse(SecuredUpload.isValidText("ruby ", allowed));
assertFalse(SecuredUpload.isValidText("/ruby", allowed));
- assertFalse(SecuredUpload.isValidText("processbuilder", allowed)); // Groovy
- assertFalse(SecuredUpload.isValidText("function", allowed)); // Groovy
- assertFalse(SecuredUpload.isValidText("class", allowed)); // Groovy
+ assertFalse(SecuredUpload.isValidText("process", allowed));
+ assertFalse(SecuredUpload.isValidText("function", allowed));
+ assertFalse(SecuredUpload.isValidText("class", allowed));
} catch (IOException e) {
fail(String.format("IOException occured : %s", e.getMessage()));