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 7eec89d Fixed: Secure the uploads (OFBIZ-12080)
7eec89d is described below
commit 7eec89d523b4ee3530a8e1de07f4544c9aba55b2
Author: Jacques Le Roux <[email protected]>
AuthorDate: Mon Feb 21 18:01:18 2022 +0100
Fixed: Secure the uploads (OFBIZ-12080)
Fixes deniedWebShellTokens tests
---
framework/security/config/security.properties | 6 +++---
.../java/org/apache/ofbiz/security/SecurityUtilTest.java | 16 +++++++++-------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index 397c0c3..8ad620b 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -249,11 +249,11 @@ 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=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,\
-
ifconfig,route,crontab,netstat,uname,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost
+ python,perl ,/perl,ruby
,/ruby,process,function,class,InputStream,to_server,wget ,\
+ ifconfig,route,crontab,netstat,uname
,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost
#-- Max line length for uploaded files, by default 10000
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 7b212dd..4cf8e37 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,11 +59,11 @@ public class SecurityUtilTest {
@Test
public void webShellTokensTesting() {
// Currently used
- //
freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@ page,\
+ //
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,\
- //
ifconfig,route,crontab,netstat,uname,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|
+ // python,perl ,/perl,ruby
,/ruby,process,function,class,InputStream,to_server,wget ,\
+ //
ifconfig,route,crontab,netstat,uname,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost
try {
List<String> allowed = new ArrayList<>();
@@ -82,6 +82,8 @@ public class SecurityUtilTest {
assertFalse(SecuredUpload.isValidText("taglib", allowed));
assertFalse(SecuredUpload.isValidText("<prefix", allowed));
assertFalse(SecuredUpload.isValidText("<%@ page", allowed));
+ assertFalse(SecuredUpload.isValidText("<?php", allowed));
+ assertFalse(SecuredUpload.isValidText("exec(", allowed));
assertFalse(SecuredUpload.isValidText("%eval", allowed));
assertFalse(SecuredUpload.isValidText("@eval", allowed));
@@ -114,23 +116,23 @@ 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("wget ", 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, ~3GB) with this token in
+ assertFalse(SecuredUpload.isValidText("uname ", allowed));
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
+ // ip, ls, nc, ip, cat and pwd can't be used, too short
assertFalse(SecuredUpload.isValidText("\"cmd\"", allowed));
assertFalse(SecuredUpload.isValidText("*cmd|", allowed));
assertFalse(SecuredUpload.isValidText("+cmd|", allowed));
assertFalse(SecuredUpload.isValidText("=cmd|", allowed));
+ assertFalse(SecuredUpload.isValidText("localhost", allowed));
} catch (IOException e) {
fail(String.format("IOException occured : %s", e.getMessage()));
