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 17:28:35 UTC

[ofbiz-framework] branch release22.01 updated: Fixed: Stored XSS in webappPath parameter from content/control/EditWebSite (OFBIZ-12584)

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch release22.01
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/release22.01 by this push:
     new cc468b1  Fixed: Stored XSS in webappPath parameter from content/control/EditWebSite (OFBIZ-12584)
cc468b1 is described below

commit cc468b125f5eaf2b0727345c61b9b5f3d64824c7
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Sun Feb 27 18:24:20 2022 +0100

    Fixed: Stored XSS in webappPath parameter from content/control/EditWebSite (OFBIZ-12584)
    
    Adds <<",","+",',','+'>> to deniedWebShellTokens as an obviously non satisfying
    (because images may contain those strings, I checked) temporary solution before
    looking at Freemarker::WhitelistMemberAccessPolicy as suggested by Matei
    
    Thanks to Matei "Mal" Badanoiu for reporting this post-auth vulnerabily
---
 framework/security/config/security.properties                      | 5 +++--
 .../src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java  | 7 +++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index 3d5d59e..4b52bfd 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -260,8 +260,9 @@ 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,<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
-
+                     ifconfig,route,crontab,netstat,uname ,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost,\
+                     ",","+",',','+'
+#-- Last line is a non satisfying (because images may contain those strings) temporary solution before looking at Freemarker::WhitelistMemberAccessPolicy
 
 #-- Max line length for uploaded files, by default 10000
 maxLineLength=
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 3f76ace..2854bf6 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
@@ -64,6 +64,9 @@ public class SecurityUtilTest {
         // 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
+        // ",","+",',','+'
+        // Last line is a non satisfying (because images may contain those strings) temporary solution before looking at
+        // Freemarker::WhitelistMemberAccessPolicy
 
         try {
             List<String> allowed = new ArrayList<>();
@@ -136,6 +139,10 @@ public class SecurityUtilTest {
             assertFalse(SecuredUpload.isValidText("=cmd|", allowed));
             assertFalse(SecuredUpload.isValidText("localhost", allowed));
 
+            assertFalse(SecuredUpload.isValidText("\",\"", allowed));
+            assertFalse(SecuredUpload.isValidText("\"+\"", allowed));
+            assertFalse(SecuredUpload.isValidText("','", allowed));
+            assertFalse(SecuredUpload.isValidText("'+'", allowed));
         } catch (IOException e) {
             fail(String.format("IOException occured : %s", e.getMessage()));
         }