[ofbiz-framework] branch trunk updated: Fixed: Secure the uploads (OFBIZ-12080)

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 a0e8a23  Fixed: Secure the uploads (OFBIZ-12080)
a0e8a23 is described below

commit a0e8a23a18edeef9d5d203841ab73234f0d2f6bf
Author: Jacques Le Roux <[hidden email]>
AuthorDate: Wed Dec 2 09:53:23 2020 +0100

    Fixed: Secure the uploads (OFBIZ-12080)
   
    validateUploadedFile() fixes another case reported in August by Harshit
---
 .../ofbiz/content/ContentManagementServices.java   | 46 ++++++++++++++++++----
 1 file changed, 38 insertions(+), 8 deletions(-)

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 1bf8704..a8d4ac0 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
@@ -18,6 +18,8 @@
  *******************************************************************************/
 package org.apache.ofbiz.content;
 
+import java.io.File;
+import java.io.IOException;
 import java.math.BigDecimal;
 import java.nio.ByteBuffer;
 import java.sql.Timestamp;
@@ -30,6 +32,7 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.imaging.ImageReadException;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.StringUtil;
 import org.apache.ofbiz.base.util.UtilDateTime;
@@ -256,7 +259,7 @@ public class ContentManagementServices {
         }
         // Do update and create permission checks on Content if warranted.
 
-        context.put("skipPermissionCheck", null);  // Force check here
+        context.put("skipPermissionCheck", null); // Force check here
         boolean contentExists = true;
         if (Debug.infoOn()) {
             Debug.logInfo("in persist... contentTypeId:" + contentTypeId + " dataResourceTypeId:" + dataResourceTypeId + " contentId:"
@@ -268,7 +271,9 @@ public class ContentManagementServices {
             } else {
                 try {
                     GenericValue val = EntityQuery.use(delegator).from("Content").where("contentId", contentId).queryOne();
-                    if (val == null) contentExists = false;
+                    if (val == null) {
+                        dataResourceExists = false;
+                    }
                 } catch (GenericEntityException e) {
                     return ServiceUtil.returnError(e.toString());
                 }
@@ -406,8 +411,7 @@ public class ContentManagementServices {
     }
 
     /**
-     * Service for update publish sites with a ContentRole that will tie them to the passed
-     * in party.
+     * Service for update publish sites with a ContentRole that will tie them to the passed in party.
      */
     public static Map<String, Object> updateSiteRoles(DispatchContext dctx, Map<String, ? extends Object> context) {
         LocalDispatcher dispatcher = dctx.getDispatcher();
@@ -531,6 +535,12 @@ public class ContentManagementServices {
         Delegator delegator = dctx.getDelegator();
         LocalDispatcher dispatcher = dctx.getDispatcher();
         Map<String, Object> context = UtilMisc.makeMapWritable(rcontext);
+
+        String errorMessage = validateUploadedFile(dctx, context);
+        if (errorMessage != null) {
+            return ServiceUtil.returnError(errorMessage);
+        }
+
         Map<String, Object> result = new HashMap<>();
         Map<String, Object> newDrContext = new HashMap<>();
         GenericValue dataResource = delegator.makeValue("DataResource");
@@ -750,7 +760,7 @@ public class ContentManagementServices {
             } else {
                 if (fromDate != null) {
                     // for now, will assume that any error is due to non-existence - ignore
-                    //return ServiceUtil.returnError(e.toString());
+                    // return ServiceUtil.returnError(e.toString());
                     try {
                         Debug.logInfo("updateSiteRoles, serviceContext(2):" + serviceContext, MODULE);
                         Map<String, Object> newContext = new HashMap<>();
@@ -981,7 +991,6 @@ public class ContentManagementServices {
         return result;
     }
 
-
     /**
      * This service changes the contentTypeId of the current content and its children depending on the pageMode.
      * if pageMode == "outline" then if the contentTypeId of children is not "OUTLINE_NODE" or "PAGE_NODE"
@@ -1542,8 +1551,8 @@ public class ContentManagementServices {
         return result;
     }
 
-    public static Map<String, Object> followNodeChildrenMethod(GenericValue content, LocalDispatcher dispatcher, String serviceName, Map<String,
-            Object> context) throws GenericEntityException, GenericServiceException {
+    public static Map<String, Object> followNodeChildrenMethod(GenericValue content, LocalDispatcher dispatcher, String serviceName,
+            Map<String, Object> context) throws GenericEntityException, GenericServiceException {
         Map<String, Object> result = null;
         String contentId = content.getString("contentId");
         List<String> contentAssocTypeIdList = UtilGenerics.cast(context.get("contentAssocTypeIdList"));
@@ -1639,4 +1648,25 @@ public class ContentManagementServices {
         }
         return result;
     }
+
+    private static String validateUploadedFile(DispatchContext dctx, Map<String, ? extends Object> context) {
+        Delegator delegator = dctx.getDelegator();
+        Locale locale = (Locale) context.get("locale");
+        String objectInfo = (String) context.get("objectInfo");
+        String errorMessage = null;
+        if (!UtilValidate.isEmpty(objectInfo)) {
+            File file = new File(objectInfo);
+            if (file.isFile()) {
+                try {
+                    if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(objectInfo, "All", delegator)) {
+                        errorMessage = UtilProperties.getMessage("SecurityUiLabels", "SupportedFileFormatsIncludingSvg", locale);
+                    }
+                } catch (ImageReadException | IOException e) {
+                    errorMessage = UtilProperties.getMessage(RESOURCE, "ContentUnableToOpenFileForWriting", UtilMisc.toMap("fileName",
+                            objectInfo), locale);
+                }
+            }
+        }
+        return errorMessage;
+    }
 }