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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

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

jleroux@apache.org
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-plugins.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 0718b63  * Fixed: Secure the uploads (OFBIZ-12080)
0718b63 is described below

commit 0718b63544af02815fcf92f50e509b0db5ee2b67
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.
---
 .../java/org/apache/ofbiz/birt/flexible/BirtServices.java     |  2 +-
 .../src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java   | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 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 01551c4..77ce2ee 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
@@ -772,7 +772,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(RES_ERROR, "BirtErrorInuploadRptDesignNoFile", locale));
         }
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 4ed4708..74ea5b3 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;
 
@@ -2257,12 +2258,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};