[ofbiz-framework] branch release17.12 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 release17.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/release17.12 by this push:
     new 9d3631e  Fixed: Secure the uploads (OFBIZ-12080)
9d3631e is described below

commit 9d3631e278773b9a92ebc9211bb24871ce1f836b
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
   
    Conflicts handled by hand
    applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
---
 .../ofbiz/content/ContentManagementServices.java   | 71 ++++++++++++++++------
 1 file changed, 52 insertions(+), 19 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 1ff7829..67713ca 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;
@@ -29,6 +31,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;
@@ -260,7 +263,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:" + contentId + " dataResourceId:" + dataResourceId, module);
@@ -271,7 +274,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());
                 }
@@ -403,9 +408,8 @@ 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();
         Delegator delegator = dctx.getDelegator();
@@ -529,17 +533,24 @@ public class ContentManagementServices {
       return result;
     }
 
-    public static Map<String, Object> persistDataResourceAndDataMethod(DispatchContext dctx, Map<String, ? extends Object> rcontext) throws GenericServiceException, GenericEntityException, Exception {
-      Delegator delegator = dctx.getDelegator();
-      LocalDispatcher dispatcher = dctx.getDispatcher();
-      Map<String, Object> context = UtilMisc.makeMapWritable(rcontext);
-      Map<String, Object> result = new HashMap<String, Object>();
-      Map<String, Object> newDrContext = new HashMap<String, Object>();
-      GenericValue dataResource = delegator.makeValue("DataResource");
-      dataResource.setPKFields(context);
-      dataResource.setNonPKFields(context);
-      dataResource.setAllFields(context, false, "dr", null);
-      context.putAll(dataResource);
+    public static Map<String, Object> persistDataResourceAndDataMethod(DispatchContext dctx, Map<String, ? extends Object> rcontext)
+            throws GenericServiceException, GenericEntityException {
+        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");
+        dataResource.setPKFields(context);
+        dataResource.setNonPKFields(context);
+        dataResource.setAllFields(context, false, "dr", null);
+        context.putAll(dataResource);
 
       GenericValue electronicText = delegator.makeValue("ElectronicText");
       electronicText.setPKFields(context);
@@ -737,7 +748,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<String, Object>();
@@ -962,7 +973,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"
@@ -1492,7 +1502,9 @@ 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.checkList(context.get("contentAssocTypeIdList"));
@@ -1586,4 +1598,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;
+    }
 }