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
commit 34d68d5f6f9037323f73f7780d2d3ecd6b030d9e
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Sun Feb 27 10:14:59 2022 +0100
Improved: Reflected XSS in content component (OFBIZ-11840)
This was already fixed but we can do better by using
SecuredUpload::isValidText
Also grabs some tokens to security.properties::deniedWebShellTokens from
DataEvents.java, and adds "alert" even it's harmless as is, as it's often
used
by attackers to demonstrate issues, so will earn time.
Though an improvement, I backport as it's related to security
Conflicts handled by hand in data/DataEvents.java
---
.../java/org/apache/ofbiz/content/data/DataEvents.java | 16 ++++++++++++----
framework/security/config/security.properties | 13 ++++++++++---
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git
a/applications/content/src/main/java/org/apache/ofbiz/content/data/DataEvents.java
b/applications/content/src/main/java/org/apache/ofbiz/content/data/DataEvents.java
index 1c81434..1c233ca 100644
---
a/applications/content/src/main/java/org/apache/ofbiz/content/data/DataEvents.java
+++
b/applications/content/src/main/java/org/apache/ofbiz/content/data/DataEvents.java
@@ -21,6 +21,7 @@ package org.apache.ofbiz.content.data;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.util.Collections;
import java.util.Locale;
import java.util.Map;
@@ -41,6 +42,7 @@ import org.apache.ofbiz.entity.GenericEntityException;
import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.entity.util.EntityQuery;
import org.apache.ofbiz.entity.util.EntityUtilProperties;
+import org.apache.ofbiz.security.SecuredUpload;
import org.apache.ofbiz.service.GenericServiceException;
import org.apache.ofbiz.service.LocalDispatcher;
import org.apache.ofbiz.service.ServiceUtil;
@@ -82,10 +84,16 @@ public class DataEvents {
// get the permission service required for streaming data; default is
always the genericContentPermission
String permissionService =
EntityUtilProperties.getPropertyValue("content", "stream.permission.service",
"genericContentPermission", delegator);
- // This is counterintuitive but it works, for OFBIZ-11840
- // It could be improved by checking for possible events associated
with svg
- // As listed at
https://portswigger.net/web-security/cross-site-scripting/cheat-sheet
- if (contentId.contains("<svg")) {
+ // For OFBIZ-11840, checks if a webshell is not uploaded
+ // It's counterintuitive to return success but it makes sense if you
thing about it.
+ // It simply returns a blank screen.
+ try {
+ if (!SecuredUpload.isValidText(contentId,
Collections.emptyList())) {
+ Debug.logError("================== Not saved for security
reason ==================", MODULE);
+ return "success";
+ }
+ } catch (IOException e) {
+ Debug.logError("================== Not saved for security reason
==================", MODULE);
return "success";
}
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index 9614d7f..47f83e1 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -215,15 +215,22 @@
deniedFileExtensions=html,htm,php,php1,php2,hph3,php4,php5,php6,php7,phps,asp,as
#-- people may like to allow more than what is allowed OOTB
#-- As it name says, allowAllUploads opens all possibilities
allowAllUploads=
+
#--
-#-- List of denied tokens often part of webshells. Note that, for now at
least, those are supposed to be used on a *nix system
+#-- List of denied tokens often part of webshells. Note that, for now at
least, most are supposed to be used on a *nix system
#-- TODO.... to be continued with known webshell contents... a complete allow
list is impossible anyway...
+#--
+#-- It could notably be improved by checking for all Javascripts payloads.
+#-- As listed at
https://portswigger.net/web-security/cross-site-scripting/cheat-sheet,
+#-- at 2022-02-25 there are 8929 of them considering all tags, all events and
all browsers...!
+#--
#-- "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=java.,beans,freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@
page,<?php,exec(,\
+deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@
page,<?php,exec(,alert(,\
%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,\
+ chmod,mkdir,fopen,fclose,new
file,upload,getfilename,download,getoutputstring,readfile,iframe,object,embed,<svg
,\
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
