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/04 14:04:57 UTC
[ofbiz-framework] branch trunk updated: Fixed: Remote Code Execution (File Upload) Vulnerability (OFBIZ-11948)
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
The following commit(s) were added to refs/heads/trunk by this push:
new 036a1fd Fixed: Remote Code Execution (File Upload) Vulnerability (OFBIZ-11948)
036a1fd is described below
commit 036a1fde297daf6e18ccd17d5f3a21bb0a4c0ecf
Author: Jacques Le Roux <ja...@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));