You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2022/02/27 12:40:51 UTC
[ofbiz-framework] branch release18.12 updated (6da27fd -> 247b871)
This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a change to branch release18.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.
from 6da27fd Fixed: Secure the uploads (OFBIZ-12080)
new 34d68d5 Improved: Reflected XSS in content component (OFBIZ-11840)
new 247b871 Fixed: Stored XSS in webappPath parameter from content/control/EditWebSite (OFBIZ-12584)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../org/apache/ofbiz/content/data/DataEvents.java | 16 ++++++++++----
framework/security/config/security.properties | 13 ++++++++---
.../ofbiz/service/engine/EntityAutoEngine.java | 25 +++++++++++++++++++++-
3 files changed, 46 insertions(+), 8 deletions(-)
[ofbiz-framework] 01/02: Improved: Reflected XSS in content component (OFBIZ-11840)
Posted by jl...@apache.org.
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 <ja...@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
[ofbiz-framework] 02/02: Fixed: Stored XSS in webappPath parameter from content/control/EditWebSite (OFBIZ-12584)
Posted by jl...@apache.org.
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 247b8713ba2e37b8dd6a8afb60425005e32e0bef
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Sun Feb 27 13:35:48 2022 +0100
Fixed: Stored XSS in webappPath parameter from content/control/EditWebSite (OFBIZ-12584)
A user with rights to modify and/or create websites may insert malicious HTML
elements in the “webappPath” parameter from content/control/EditWebSite
resulting in XSS.
In order to trigger the XSS a victim needs to navigate to main page of the
modified website (eg webpos or ecommerce) and interact with the malicious
HTML elements (eg trigger the “onmouseover” event by navigating with the mouse
over the “form” and/or “a” tags).
Thanks to Matei "Mal" Badanoiu for reporting this post-auth vulnerabily
Conflicts handled by hand in EntityAutoEngine.java
---
.../ofbiz/service/engine/EntityAutoEngine.java | 25 +++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/engine/EntityAutoEngine.java b/framework/service/src/main/java/org/apache/ofbiz/service/engine/EntityAutoEngine.java
index 208ea7a..cff8d9e 100644
--- a/framework/service/src/main/java/org/apache/ofbiz/service/engine/EntityAutoEngine.java
+++ b/framework/service/src/main/java/org/apache/ofbiz/service/engine/EntityAutoEngine.java
@@ -18,6 +18,8 @@
*/
package org.apache.ofbiz.service.engine;
+import java.io.IOException;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
@@ -36,6 +38,7 @@ import org.apache.ofbiz.entity.finder.PrimaryKeyFinder;
import org.apache.ofbiz.entity.model.ModelEntity;
import org.apache.ofbiz.entity.model.ModelField;
import org.apache.ofbiz.entity.util.EntityQuery;
+import org.apache.ofbiz.security.SecuredUpload;
import org.apache.ofbiz.service.DispatchContext;
import org.apache.ofbiz.service.GenericServiceException;
import org.apache.ofbiz.service.ModelParam;
@@ -70,7 +73,10 @@ public final class EntityAutoEngine extends GenericAsyncEngine {
@Override
public Map<String, Object> runSync(String localName, ModelService modelService, Map<String, Object> parameters) throws GenericServiceException {
// static java service methods should be: public Map<String, Object> methodName(DispatchContext dctx, Map<String, Object> context)
- DispatchContext dctx = dispatcher.getLocalContext(localName);
+ if (!isValidText(parameters)) {
+ return ServiceUtil.returnError("Not saved for security reason!");
+ }
+ DispatchContext dctx = getDispatcher().getLocalContext(localName);
Locale locale = (Locale) parameters.get("locale");
Map<String, Object> result = ServiceUtil.returnSuccess();
@@ -578,4 +584,21 @@ public final class EntityAutoEngine extends GenericAsyncEngine {
Map<String, Object> result = ServiceUtil.returnSuccess(UtilProperties.getMessage("ServiceUiLabels", "EntityExpiredSuccessfully", UtilMisc.toMap("label", modelEntity.getTitle()), locale));
return result;
}
+
+ private static boolean isValidText(Map<String, Object> parameters) {
+ // TODO maybe more parameters will be needed in future...
+ String parameter = (String) parameters.get("webappPath");
+ if (parameter != null) {
+ try {
+ if (!SecuredUpload.isValidText(parameter, Collections.emptyList())) {
+ Debug.logError("================== Not saved for security reason ==================", MODULE);
+ return false;
+ }
+ } catch (IOException e) {
+ Debug.logError("================== Not saved for security reason ==================", MODULE);
+ return false;
+ }
+ }
+ return true;
+ }
}