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


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

commit 49b96e47cccc17f904f489c6d600532cf51ee8bf
Author: Jacques Le Roux <[hidden email]>
AuthorDate: Tue Dec 1 08:49:25 2020 +0100

    * Fixed: Secure the uploads (OFBIZ-12080)
   
    2020/08/10 the OFBiz security team received a security report by Harshit Shukla
    <[hidden email]>, roughly it was (quoting part of it to simplify):
   
    <<I have identified a Remote Code Execution (RCE) Vulnerability. The reason
    behind this RCE is lack of file extension check at
    catalog/control/UploadCategoryImage?productCategoryId=CATALOG1_BEST_SELL&pload_file_type=category>>
   
    Using this post-auth RCE in OFBiz demos, Harshit was able to get some AWS
    credentials by uploading a webshell (based on [0]).
    By security, it was then decided by the Infra and OFBiz security teams to shut
    down the demos.
   
    After discussing the elements reported with Mark J Cox (VP of ASF security team)
    we in common decided that no CVE was necessary.
   
    # Conflicts handled by hand:
    # birt/src/main/java/org/apache/ofbiz/birt/flexible/BirtServices.java
---
 .../java/org/apache/ofbiz/birt/flexible/BirtServices.java     |  2 +-
 .../org/apache/ofbiz/cmssite/multisite/WebSiteFilter.java     | 10 +++++-----
 .../src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java   | 11 ++++++++++-
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/birt/src/main/java/org/apache/ofbiz/birt/flexible/BirtServices.java b/birt/src/main/java/org/apache/ofbiz/birt/flexible/BirtServices.java
index df9dc22..4547567 100644
--- a/birt/src/main/java/org/apache/ofbiz/birt/flexible/BirtServices.java
+++ b/birt/src/main/java/org/apache/ofbiz/birt/flexible/BirtServices.java
@@ -749,7 +749,7 @@ public class BirtServices {
             // user file is deleted straight away to prevent the use of the report as script entry (security)
             Path path = Paths.get(nameTempRpt);
             Files.deleteIfExists(path);
-        } catch (Exception e) {
+        } catch (DesignFileException | IOException e) {
             Debug.logError(e, module);
             return ServiceUtil.returnError(UtilProperties.getMessage(resource_error, "BirtErrorInuploadRptDesignNoFile", locale));
         }
diff --git a/cmssite/src/main/java/org/apache/ofbiz/cmssite/multisite/WebSiteFilter.java b/cmssite/src/main/java/org/apache/ofbiz/cmssite/multisite/WebSiteFilter.java
index d49c186..f865ca2 100644
--- a/cmssite/src/main/java/org/apache/ofbiz/cmssite/multisite/WebSiteFilter.java
+++ b/cmssite/src/main/java/org/apache/ofbiz/cmssite/multisite/WebSiteFilter.java
@@ -51,7 +51,7 @@ import org.apache.ofbiz.webapp.stats.VisitHandler;
 // Used to filter website on the basis of hosted pathAlias.
 public class WebSiteFilter implements Filter {
 
-    public static final String MODULE = WebSiteFilter.class.getName();
+    public static final String module = WebSiteFilter.class.getName();
 
     protected FilterConfig m_config = null;
 
@@ -85,7 +85,7 @@ public class WebSiteFilter implements Filter {
                 webSite = EntityQuery.use(delegator).from("WebSite").where("isDefault", "Y").cache().queryFirst();
             }
         } catch (GenericEntityException e) {
-            Debug.logError(e, MODULE);
+            Debug.logError(e, module);
         }
         if (webSite != null) {
             webSiteId = webSite.getString("webSiteId");
@@ -93,7 +93,7 @@ public class WebSiteFilter implements Filter {
             try {
                 productStore = webSite.getRelatedOne("ProductStore", false);
             } catch (GenericEntityException e) {
-                Debug.logError(e, MODULE);
+                Debug.logError(e, module);
             }
 
             String newLocale = request.getParameter("newLocale");
@@ -120,7 +120,7 @@ public class WebSiteFilter implements Filter {
                     try {
                         cart.setCurrency(dispatcher, productStore.getString("defaultCurrencyUomId"));
                     } catch (CartItemModifyException e) {
-                        Debug.logError(e, MODULE);
+                        Debug.logError(e, module);
                     }
                 }
                 session.removeAttribute("webSiteId");
@@ -149,7 +149,7 @@ public class WebSiteFilter implements Filter {
         try {
             security = SecurityFactory.getInstance(delegator);
         } catch (SecurityConfigurationException e) {
-            Debug.logError(e, MODULE);
+            Debug.logError(e, module);
         }
         request.setAttribute("delegator", delegator);
         request.setAttribute("dispatcher", dispatcher);
diff --git a/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java b/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java
index 9a09812..56d46c1 100644
--- a/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java
+++ b/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java
@@ -40,6 +40,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
+import javax.imageio.ImageIO;
 import javax.swing.table.AbstractTableModel;
 import javax.swing.table.TableModel;
 
@@ -2267,12 +2268,20 @@ public class EbayStore {
 
                 // Upload image to ofbiz path /runtime/tmp .
                 ByteBuffer byteWrap = (ByteBuffer) context.get("imageData");
-                File file = new File(System.getProperty("ofbiz.home"), "runtime" + File.separator + "tmp" + File.separator + imageFileName);
+                String fileToCheck = System.getProperty("ofbiz.home"), "runtime" + File.separator + "tmp" + File.separator + imageFileName;
+                File file = new File(fileToCheck);
                 FileOutputStream fileOutputStream = new FileOutputStream(file, false);
                 FileChannel wChannel = fileOutputStream.getChannel();
                 wChannel.write(byteWrap);
                 wChannel.close();
                 fileOutputStream.close();
+                String fileToCheck = imageServerPath + "/" + newFileLocation + "." + imgExtension;
+                ImageIO.write(bufNewImg, imgExtension, new File(fileToCheck));
+                if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(fileToCheck, "Image")) {
+                    String errorMessage = UtilProperties.getMessage("SecurityUiLabels", "SupportedImageFormats", locale);
+                    return ServiceUtil.returnError(errorMessage);
+                }
+                
 
                 // Set path file picture to api and set picture details.
                 String [] pictureFiles = {System.getProperty("ofbiz.home") + File.separator + "runtime" + File.separator + "tmp" + File.separator + imageFileName};