This is an automated email from the ASF dual-hosted git repository. jleroux pushed a commit to branch release18.12 in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release18.12 by this push: new c4bd082 Fixed: Secure the uploads (OFBIZ-12080) c4bd082 is described below commit c4bd0823595a71e8373a4803bfd4af73912a951b Author: Jacques Le Roux <jacques.le.r...@les7arts.com> AuthorDate: Thu Mar 17 13:37:57 2022 +0100 Fixed: Secure the uploads (OFBIZ-12080) Changes name="upload_file_type_bogus" in EditCategory.ftl and EditProductContent.ftl to name="up_load_file_type_bogus" The last line of deniedWebShellTokens in SecuredUpload and similarly in SecurityUtilTest classes should no have a comma and anti-slash as previous lines. Also logs when an image contains one of the token in deniedWebShellTokens --- applications/product/template/category/EditCategory.ftl | 6 +++--- applications/product/template/product/EditProductContent.ftl | 10 +++++----- framework/security/config/security.properties | 2 +- .../src/main/java/org/apache/ofbiz/security/SecuredUpload.java | 6 +++++- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/applications/product/template/category/EditCategory.ftl b/applications/product/template/category/EditCategory.ftl index 50c0b8b..f45fb7f 100644 --- a/applications/product/template/category/EditCategory.ftl +++ b/applications/product/template/category/EditCategory.ftl @@ -219,9 +219,9 @@ function insertImageName(type,nameValue) { <input type="file" size="50" name="fname"/> <br /> <span> - <label><input type="radio" name="upload_file_type_bogus" value="category" checked="checked" onclick='setUploadUrl("<@ofbizUrl>UploadCategoryImage?productCategoryId=${productCategoryId}&upload_file_type=category</@ofbizUrl>");'/>${uiLabelMap.ProductCategoryImageUrl}</label> - <label><input type="radio" name="upload_file_type_bogus" value="linkOne" onclick='setUploadUrl("<@ofbizUrl>UploadCategoryImage?productCategoryId=${productCategoryId}&upload_file_type=linkOne</@ofbizUrl>");'/>${uiLabelMap.ProductLinkOneImageUrl}</label> - <label><input type="radio" name="upload_file_type_bogus" value="linkTwo"onclick='setUploadUrl("<@ofbizUrl>UploadCategoryImage?productCategoryId=${productCategoryId}&upload_file_type=linkTwo</@ofbizUrl>");'/>${uiLabelMap.ProductLinkTwoImageUrl}</label> + <label><input type="radio" name="up_load_file_type_bogus" value="category" checked="checked" onclick='setUploadUrl("<@ofbizUrl>UploadCategoryImage?productCategoryId=${productCategoryId}&upload_file_type=category</@ofbizUrl>");'/>${uiLabelMap.ProductCategoryImageUrl}</label> + <label><input type="radio" name="up_load_file_type_bogus" value="linkOne" onclick='setUploadUrl("<@ofbizUrl>UploadCategoryImage?productCategoryId=${productCategoryId}&upload_file_type=linkOne</@ofbizUrl>");'/>${uiLabelMap.ProductLinkOneImageUrl}</label> + <label><input type="radio" name="up_load_file_type_bogus" value="linkTwo"onclick='setUploadUrl("<@ofbizUrl>UploadCategoryImage?productCategoryId=${productCategoryId}&upload_file_type=linkTwo</@ofbizUrl>");'/>${uiLabelMap.ProductLinkTwoImageUrl}</label> </span> <input type="submit" class="smallSubmit" value="${uiLabelMap.ProductUploadImage}"/> </td></tr> diff --git a/applications/product/template/product/EditProductContent.ftl b/applications/product/template/product/EditProductContent.ftl index 5d05b7c..7fafff9 100644 --- a/applications/product/template/product/EditProductContent.ftl +++ b/applications/product/template/product/EditProductContent.ftl @@ -192,11 +192,11 @@ under the License. </td> <td> </td> <td width="80%" colspan="4" valign="top"> - <label><input type="radio" name="upload_file_type_bogus" value="small" onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=small</@ofbizUrl>");'/>${uiLabelMap.CommonSmall}</label> - <label><input type="radio" name="upload_file_type_bogus" value="medium" onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=medium</@ofbizUrl>");'/>${uiLabelMap.CommonMedium}</label> - <label><input type="radio" name="upload_file_type_bogus" value="large"onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=large</@ofbizUrl>");'/>${uiLabelMap.CommonLarge}</label> - <label><input type="radio" name="upload_file_type_bogus" value="detail" onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=detail</@ofbizUrl>");'/>${uiLabelMap.CommonDetail}</label> - <label><input type="radio" name="upload_file_type_bogus" value="original" checked="checked" onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=original</@ofbizUrl>");'/>${uiLabelMap.ProductOriginal}</label> + <label><input type="radio" name="up_load_file_type_bogus" value="small" onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=small</@ofbizUrl>");'/>${uiLabelMap.CommonSmall}</label> + <label><input type="radio" name="up_load_file_type_bogus" value="medium" onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=medium</@ofbizUrl>");'/>${uiLabelMap.CommonMedium}</label> + <label><input type="radio" name="up_load_file_type_bogus" value="large"onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=large</@ofbizUrl>");'/>${uiLabelMap.CommonLarge}</label> + <label><input type="radio" name="up_load_file_type_bogus" value="detail" onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=detail</@ofbizUrl>");'/>${uiLabelMap.CommonDetail}</label> + <label><input type="radio" name="up_load_file_type_bogus" value="original" checked="checked" onclick='setUploadUrl("<@ofbizUrl>UploadProductImage?productId=${productId}&upload_file_type=original</@ofbizUrl>");'/>${uiLabelMap.ProductOriginal}</label> <input type="submit" class="smallSubmit" value="${uiLabelMap.ProductUploadImage}"/> </td> </tr> diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties index 55474de..ff57641 100644 --- a/framework/security/config/security.properties +++ b/framework/security/config/security.properties @@ -232,7 +232,7 @@ deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,<form,<jsp: %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,assign,webappPath,\ - ifconfig,route,crontab,netstat,uname ,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost,\ + ifconfig,route,crontab,netstat,uname ,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost #-- 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 b1225dc..0b18420 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 @@ -677,7 +677,11 @@ public class SecuredUpload { } private static boolean isValid(String content, String string, List<String> allowed) { - return !content.toLowerCase().contains(string) || allowed.contains(string); + boolean isOK = !content.toLowerCase().contains(string) || allowed.contains(string); + if (!isOK) { + Debug.logInfo("############### BEWARE, Image contains " + string, MODULE); + } + return isOK; } private static void deleteBadFile(String fileToCheck) {