[ofbiz-framework] 03/03: Fixed: Prevent post-Auth vulnerability: FreeMarker Bypass (OFBIZ-12582)

[jleroux]

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 f65410ba1d6b13ea5edfb07aa43b65a8f296cb89
Author: Jacques Le Roux <jacques.le.r...@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 <aware...@gmail.com> for reporting this post-auth 
vulnerabily
    
    Conflicts handled by hand in ContentManagementServices.java
---
 .../apache/ofbiz/content/ContentManagementServices.java  | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

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 c185266..368cc63 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.LinkedList;
@@ -49,6 +50,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;
@@ -143,6 +145,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