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/23 10:59:16 UTC
[ofbiz-framework] branch release22.01 updated (6e54f7d -> 16c8afe)
This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a change to branch release22.01
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.
from 6e54f7d Fixed: Secure the uploads (OFBIZ-12080)
new f1b9671 Fixed: Trivial change in ProgramExport.groovy, uses the SecuredUpload import
new 7157cf3 Fixed: Secure the uploads (OFBIZ-12080)
new 16c8afe Fixed: Prevent post-Auth vulnerability: FreeMarker Bypass (OFBIZ-12582)
The 3 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:
.../ofbiz/content/ContentManagementServices.java | 18 +++++++++++++++++-
framework/security/config/security.properties | 4 ++--
.../org/apache/ofbiz/security/SecurityUtilTest.java | 8 +++++---
.../webtools/groovyScripts/entity/ProgramExport.groovy | 4 +++-
4 files changed, 27 insertions(+), 7 deletions(-)
[ofbiz-framework] 01/03: Fixed: Trivial change in ProgramExport.groovy, uses the SecuredUpload import
Posted by jl...@apache.org.
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
commit f1b967160f56d550a413784ff9fd854b929657d4
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Wed Feb 23 09:31:25 2022 +0100
Fixed: Trivial change in ProgramExport.groovy, uses the SecuredUpload import
No Functional change
Conflicts handled by hand in ProgramExport.groovy
---
framework/webtools/groovyScripts/entity/ProgramExport.groovy | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/framework/webtools/groovyScripts/entity/ProgramExport.groovy b/framework/webtools/groovyScripts/entity/ProgramExport.groovy
index 52a7a03..47923d4 100644
--- a/framework/webtools/groovyScripts/entity/ProgramExport.groovy
+++ b/framework/webtools/groovyScripts/entity/ProgramExport.groovy
@@ -17,6 +17,7 @@
* under the License.
*/
import org.apache.ofbiz.entity.GenericValue
+import org.apache.ofbiz.security.SecuredUpload
import org.codehaus.groovy.control.CompilerConfiguration
import org.codehaus.groovy.control.MultipleCompilationErrorsException
import org.codehaus.groovy.control.customizers.ImportCustomizer
@@ -78,7 +79,8 @@ def shell = new GroovyShell(loader, binding, configuration)
if (groovyProgram) {
try {
// Check if a webshell is not uploaded but allow "import"
- if (!org.apache.ofbiz.security.SecuredUpload.isValidText(groovyProgram,["import"])) {
+ if (!SecuredUpload.isValidText(groovyProgram, ["import"])) {
+ logError("================== Not executed for security reason ==================")
request.setAttribute("_ERROR_MESSAGE_", "Not executed for security reason")
return
}
[ofbiz-framework] 03/03: Fixed: Prevent post-Auth vulnerability: FreeMarker Bypass (OFBIZ-12582)
Posted by jl...@apache.org.
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
commit 16c8afe5d0c103aabd05b8237820a86eea761e1c
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Wed Feb 23 11:46:18 2022 +0100
Fixed: Prevent post-Auth vulnerability: FreeMarker Bypass (OFBIZ-12582)
By inserting malicious content in the “Text” field from
“/content/control/updateLayoutSubContent” -> “Templates”, an attacker may
perform SSTI (Server-Side Template Injection) attacks, which can leverage
FreeMarker exposed objects to bypass restrictions and obtain RCE (Remote Code
Execution).
This fixes it by calling SecuredUpload::isValidText on the “Text” field content.
I'll check that there are no other attack opportunities...
Thanks: Mal Aware <aw...@gmail.com> for reporting this post-auth vulnerabily
---
.../ofbiz/content/ContentManagementServices.java | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java b/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
index a9b2133..b273d66 100644
--- a/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
+++ b/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
@@ -23,6 +23,7 @@ import java.io.IOException;
import java.math.BigDecimal;
import java.nio.ByteBuffer;
import java.sql.Timestamp;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
@@ -50,6 +51,7 @@ import org.apache.ofbiz.entity.condition.EntityOperator;
import org.apache.ofbiz.entity.model.ModelUtil;
import org.apache.ofbiz.entity.util.EntityQuery;
import org.apache.ofbiz.entity.util.EntityUtil;
+import org.apache.ofbiz.security.SecuredUpload;
import org.apache.ofbiz.security.Security;
import org.apache.ofbiz.service.DispatchContext;
import org.apache.ofbiz.service.GenericServiceException;
@@ -146,6 +148,20 @@ public class ContentManagementServices {
Map<String, Object> context = UtilMisc.makeMapWritable(rcontext);
Locale locale = (Locale) context.get("locale");
+ // Check if a webshell is not uploaded
+ String textData = (String) context.get("textData");
+ if (UtilValidate.isNotEmpty(textData)) {
+ try {
+ if (!SecuredUpload.isValidText(textData, Collections.emptyList())) {
+ Debug.logError("================== Not saved for security reason ==================", MODULE);
+ return ServiceUtil.returnError("================== Not saved for security reason ==================");
+ }
+ } catch (IOException e) {
+ Debug.logError("================== Not saved for security reason ==================", MODULE);
+ return ServiceUtil.returnError("================== Not saved for security reason ==================");
+ }
+ }
+
// Knowing why a request fails permission check is one of the more difficult
// aspects of content management. Setting "displayFailCond" to true will
// put an html table in result.errorMessage that will show what tests were performed
@@ -176,7 +192,7 @@ public class ContentManagementServices {
if (Debug.infoOn()) {
Debug.logInfo("in persist... contentPurposeList(0):" + contentPurposeList, MODULE);
- Debug.logInfo("in persist... textData(0):" + context.get("textData"), MODULE);
+ Debug.logInfo("in persist... textData(0):" + textData, MODULE);
}
GenericValue content = delegator.makeValue("Content");
[ofbiz-framework] 02/03: Fixed: Secure the uploads (OFBIZ-12080)
Posted by jl...@apache.org.
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
commit 7157cf36818f5b11ee84d269733ccb21940c7a8b
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Wed Feb 23 09:33:17 2022 +0100
Fixed: Secure the uploads (OFBIZ-12080)
Adds some tokens in security.properties::deniedWebShellTokens
Updates SecurityUtilTest::webShellTokensTesting accordingly
---
framework/security/config/security.properties | 4 ++--
.../src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java | 8 +++++---
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index 8ad620b..d41def2 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -249,10 +249,10 @@ allowAllUploads=
#-- "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=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(,\
%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,\
- python,perl ,/perl,ruby ,/ruby,process,function,class,InputStream,to_server,wget ,\
+ 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 4cf8e37..0afa0f6 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,10 +59,10 @@ public class SecurityUtilTest {
@Test
public void webShellTokensTesting() {
// Currently used
- // 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(,\
// %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,\
- // python,perl ,/perl,ruby ,/ruby,process,function,class,InputStream,to_server,wget ,\
+ // 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
try {
@@ -72,6 +72,8 @@ public class SecurityUtilTest {
allowed = new ArrayList<>();
assertFalse(SecuredUpload.isValidText("hack.getFileName", allowed));
+ assertFalse(SecuredUpload.isValidText("java.", allowed));
+ assertFalse(SecuredUpload.isValidText("beans", allowed));
assertFalse(SecuredUpload.isValidText("freemarker", allowed));
assertFalse(SecuredUpload.isValidText("<script", allowed));
assertFalse(SecuredUpload.isValidText("javascript", allowed));
@@ -117,7 +119,7 @@ public class SecurityUtilTest {
assertFalse(SecuredUpload.isValidText("function", allowed));
assertFalse(SecuredUpload.isValidText("class", allowed));
assertFalse(SecuredUpload.isValidText("wget ", allowed));
-
+ assertFalse(SecuredUpload.isValidText("static", allowed));
assertFalse(SecuredUpload.isValidText("ifconfig", allowed));
assertFalse(SecuredUpload.isValidText("route", allowed));