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 <[email protected]>
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) {
