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
commit fe4cefd590216443720d83823906677aedcb402e
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
---
.../org/apache/ofbiz/content/data/DataEvents.java | 28 ++++++++++------------
framework/security/config/security.properties | 13 +++++++---
.../apache/ofbiz/security/SecurityUtilTest.java | 4 ++--
3 files changed, 24 insertions(+), 21 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 1a91686..9fa8c7f 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;
@@ -83,22 +85,16 @@ public class DataEvents {
String permissionService =
EntityUtilProperties.getPropertyValue("content", "stream.permission.service",
"genericContentPermission", delegator);
- // @formatter:off (prevent unwanted formatting in Eclipse)
- // For OFBIZ-11840. It's counterintuitive to return success but it
makes sense if you thing about it. It simply returns a blank screen.
- // To illustrate, only few payloads, onLoad related, are handled
because it works everytime.
- // It could be improved by checking for all payloads.
- // As listed at
https://portswigger.net/web-security/cross-site-scripting/cheat-sheet, at
2020-11-11 there are 8979 of them.
- // So a way could be to read all of them and test...
- // @formatter:on
-
- if (contentId.toLowerCase().contains("<svg")
- || contentId.toLowerCase().contains("<body")
- || contentId.toLowerCase().contains("<iframe")
- || contentId.toLowerCase().contains("<object")
- || contentId.toLowerCase().contains("<embed")
- || contentId.toLowerCase().contains("<a href='javas")
- || contentId.toLowerCase().contains("<a href=\"javas")
- || contentId.toLowerCase().contains("<script")) {
+ // 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 d41def2..3d5d59e 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -243,15 +243,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
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 0afa0f6..3f76ace 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,9 +59,9 @@ public class SecurityUtilTest {
@Test
public void webShellTokensTesting() {
// Currently used
- //
java.,beans,freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@
page,<?php,exec(,\
+ //
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
