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 b2b6250 Fixed: Remote Code Execution (File Upload) Vulnerability (OFBIZ-11948) b2b6250 is described below commit b2b6250fac491d50c1e883bdb092a4df06bf9831 Author: Jacques Le Roux <jacques.le.r...@les7arts.com> AuthorDate: Fri Feb 4 15:03:51 2022 +0100 Fixed: Remote Code Execution (File Upload) Vulnerability (OFBIZ-11948) I forgot about SecurityUtilTest::webShellTokensTesting. This fixes it. Note that I expect to simplify and remove more tokens for PHP, but I have 1st other things to do... --- framework/security/config/security.properties | 3 ++- .../apache/ofbiz/security/SecurityUtilTest.java | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties index 9eccffb..4bb2751 100644 --- a/framework/security/config/security.properties +++ b/framework/security/config/security.properties @@ -238,9 +238,10 @@ allowAllUploads= #-- eg: https://www.acunetix.com/blog/articles/detection-prevention-introduction-web-shells-part-5/ #-- "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... -deniedWebShellTokens=<%,<jsp:,<?,#!,freemarker,<script,javascript,%eval,@eval,<body>,<form,\ +deniedWebShellTokens=<%,<jsp,<?,#!,freemarker,<script,javascript,%eval,@eval,<body>,<form,\ import os,passthru,exec,shell_exec,assert,str_rot13,system,base64_decode,chmod,mkdir,\ fopen,fclose,new file,import,upload,getfilename,download,getoutputstring,readfile +#-- IMPORTANT: when you change things here you need to do accordingly in SecurityUtilTest::webShellTokensTesting-- #-- Popup last-visited time from database after user has logged in. #-- So users can know of any unauthorised access to their accounts. 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 85846f1..18727d3 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 @@ -18,6 +18,10 @@ */ package org.apache.ofbiz.security; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -25,10 +29,6 @@ import java.util.List; import org.junit.Test; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.fail; - public class SecurityUtilTest { @Test public void basicAdminPermissionTesting() { @@ -64,17 +64,18 @@ public class SecurityUtilTest { assertTrue(SecuredUpload.isValidText("hack.getFileName", allowed)); allowed = new ArrayList<>(); assertFalse(SecuredUpload.isValidText("hack.getFileName", allowed)); + + assertFalse(SecuredUpload.isValidText("<%", allowed)); + assertFalse(SecuredUpload.isValidText("<jsp", allowed)); + assertFalse(SecuredUpload.isValidText("<?", allowed)); + assertFalse(SecuredUpload.isValidText("#!", allowed)); assertFalse(SecuredUpload.isValidText("freemarker", allowed)); - assertFalse(SecuredUpload.isValidText("import=\"java", allowed)); - assertFalse(SecuredUpload.isValidText("runtime.getruntime().exec(", allowed)); - assertFalse(SecuredUpload.isValidText("<%@ page", allowed)); assertFalse(SecuredUpload.isValidText("<script", allowed)); - assertFalse(SecuredUpload.isValidText("<body>", allowed)); - assertFalse(SecuredUpload.isValidText("<form", allowed)); - assertFalse(SecuredUpload.isValidText("php", allowed)); assertFalse(SecuredUpload.isValidText("javascript", allowed)); assertFalse(SecuredUpload.isValidText("%eval", allowed)); assertFalse(SecuredUpload.isValidText("@eval", allowed)); + assertFalse(SecuredUpload.isValidText("<body>", allowed)); + assertFalse(SecuredUpload.isValidText("<form", allowed)); assertFalse(SecuredUpload.isValidText("import os", allowed)); assertFalse(SecuredUpload.isValidText("passthru", allowed)); assertFalse(SecuredUpload.isValidText("exec", allowed)); @@ -82,7 +83,6 @@ public class SecurityUtilTest { assertFalse(SecuredUpload.isValidText("assert", allowed)); assertFalse(SecuredUpload.isValidText("str_rot13", allowed)); assertFalse(SecuredUpload.isValidText("system", allowed)); - assertFalse(SecuredUpload.isValidText("phpinfo", allowed)); assertFalse(SecuredUpload.isValidText("base64_decode", allowed)); assertFalse(SecuredUpload.isValidText("chmod", allowed)); assertFalse(SecuredUpload.isValidText("mkdir", allowed));