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 e70deb4 Fixed: Secure the uploads (OFBIZ-12080) e70deb4 is described below commit e70deb4caead15d275432fe088ea79637ff73d03 Author: Jacques Le Roux <jacques.le.r...@les7arts.com> AuthorDate: Thu Mar 3 17:22:50 2022 +0100 Fixed: Secure the uploads (OFBIZ-12080) Removes non satisfying last line (because images may contain those strings) in security.properties and SecurityUtilTest.java In SecuredUpload, renames deniedFileExtensions and deniedWebShellTokens to respectively getDeniedFileExtensions and getDeniedWebShellTokens --- framework/security/config/security.properties | 2 -- .../src/main/java/org/apache/ofbiz/security/SecuredUpload.java | 8 ++++---- .../test/java/org/apache/ofbiz/security/SecurityUtilTest.java | 9 +-------- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties index e827a23..039cd5a 100644 --- a/framework/security/config/security.properties +++ b/framework/security/config/security.properties @@ -261,8 +261,6 @@ deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,<form,<jsp: chmod,mkdir,fopen,fclose,new file,upload,getfilename,download,getoutputstring,readfile,iframe,object,embed,onload,build,\ 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,\ - ",","+",',','+' -#-- Last line is a non satisfying (because images may contain those strings) temporary solution before looking at Freemarker::WhitelistMemberAccessPolicy #-- Max line length for uploaded files, by default 10000 maxLineLength= 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 8fbe8e5..fbf2a6a 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 @@ -99,8 +99,8 @@ public class SecuredUpload { // Supported file formats are *safe* PNG, GIF, TIFF, JPEG, PDF, Audio, Video, Text, and ZIP private static final String MODULE = SecuredUpload.class.getName(); - private static final List<String> DENIEDFILEEXTENSIONS = deniedFileExtensions(); - private static final List<String> DENIEDWEBSHELLTOKENS = deniedWebShellTokens(); + private static final List<String> DENIEDFILEEXTENSIONS = getDeniedFileExtensions(); + private static final List<String> DENIEDWEBSHELLTOKENS = getDeniedWebShellTokens(); private static final Integer MAXLINELENGTH = UtilProperties.getPropertyAsInteger("security", "maxLineLength", 10000); public static boolean isValidText(String content, List<String> allowed) throws IOException { @@ -688,12 +688,12 @@ public class SecuredUpload { } } - private static List<String> deniedFileExtensions() { + private static List<String> getDeniedFileExtensions() { String deniedExtensions = UtilProperties.getPropertyValue("security", "deniedFileExtensions"); return UtilValidate.isNotEmpty(deniedExtensions) ? StringUtil.split(deniedExtensions, ",") : new ArrayList<>(); } - private static List<String> deniedWebShellTokens() { + private static List<String> getDeniedWebShellTokens() { String deniedTokens = UtilProperties.getPropertyValue("security", "deniedWebShellTokens"); return UtilValidate.isNotEmpty(deniedTokens) ? StringUtil.split(deniedTokens, ",") : new ArrayList<>(); } 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 9e5ee7b..698d1c8 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 @@ -63,10 +63,7 @@ public class SecurityUtilTest { // %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,iframe,object,embed,onload,build\ // 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 - // ",","+",',','+' - // Last line is a non satisfying (because images may contain those strings) temporary solution before looking at - // Freemarker::WhitelistMemberAccessPolicy + // ifconfig,route,crontab,netstat,uname ,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost,\ try { List<String> allowed = new ArrayList<>(); @@ -144,10 +141,6 @@ public class SecurityUtilTest { assertFalse(SecuredUpload.isValidText("=cmd|", allowed)); assertFalse(SecuredUpload.isValidText("localhost", allowed)); - assertFalse(SecuredUpload.isValidText("\",\"", allowed)); - assertFalse(SecuredUpload.isValidText("\"+\"", allowed)); - assertFalse(SecuredUpload.isValidText("','", allowed)); - assertFalse(SecuredUpload.isValidText("'+'", allowed)); } catch (IOException e) { fail(String.format("IOException occured : %s", e.getMessage())); }