You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2022/02/19 11:10:29 UTC

[ofbiz-framework] 04/04: Improved: 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

commit b7f1d28142dca7dc3c7e9fd2d1e6675f6980bc7e
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Sat Feb 19 11:51:14 2022 +0100

    Improved: Secure the uploads (OFBIZ-12080)
    
    Manually merges SecuredUpload.java based on 18.12, 2nd time because of previous
    commit :/
---
 .../org/apache/ofbiz/security/SecuredUpload.java   | 48 +++++-----------------
 1 file changed, 11 insertions(+), 37 deletions(-)

diff --git a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
index aac85a0..b93320a 100644
--- a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
+++ b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
@@ -145,7 +145,6 @@ public class SecuredUpload {
                 return false;
             }
             if (DENIEDFILEEXTENSIONS.contains(extension)) {
-<<<<<<< HEAD
                 Debug.logError("This file extension is not allowed for security reason", MODULE);
                 deleteBadFile(fileToCheck);
                 return false;
@@ -157,37 +156,23 @@ public class SecuredUpload {
                 if (fileToCheck.length() > 259) {
                     Debug.logError("Uploaded file name too long", MODULE);
                 } else if (p.toString().contains(imageServerUrl.replaceAll("/", "\\\\"))) {
-=======
-            Debug.logError("This file extension is not allowed for security reason", MODULE);
-            deleteBadFile(fileToCheck);
-            return false;
-        }
-
-            // Check the file and path names
-        if (org.apache.commons.lang3.SystemUtils.IS_OS_WINDOWS) {
-            // More about that: https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
-            if (fileToCheck.length() > 259) {
-                Debug.logError("Uploaded file name too long", MODULE);
-            } else if (p.toString().contains(imageServerUrl.replaceAll("/", "\\\\"))) {
->>>>>>> 668e801ada (Improved: Secure the uploads (OFBIZ-12080))
                     // TODO check this is still useful in at least 1 case
                     if (fileName.matches("[a-zA-Z0-9-_ ()]{1,249}.[a-zA-Z0-9-_ ]{1,10}")) { // "(" and ")" for duplicates files
-                        wrongFile = false;
-                    }
-                } else if (fileName.matches("[a-zA-Z0-9-_ ]{1,249}.[a-zA-Z0-9-_ ]{1,10}")) {
                     wrongFile = false;
                 }
-            } else { // Suppose a *nix system
-                if (fileToCheck.length() > 4096) {
-                    Debug.logError("Uploaded file name too long", MODULE);
-                } else if (p.toString().contains(imageServerUrl)) {
+            } else if (fileName.matches("[a-zA-Z0-9-_ ]{1,249}.[a-zA-Z0-9-_ ]{1,10}")) {
+                wrongFile = false;
+            }
+        } else { // Suppose a *nix system
+            if (fileToCheck.length() > 4096) {
+                Debug.logError("Uploaded file name too long", MODULE);
+            } else if (p.toString().contains(imageServerUrl)) {
                     // TODO check this is still useful in at least 1 case
-                    if (fileName.matches("[a-zA-Z0-9-_ ()]{1,4086}.[a-zA-Z0-9-_ ]{1,10}")) { // "(" and ")" for duplicates files
-                        wrongFile = false;
-                    }
-                } else if (fileName.matches("[a-zA-Z0-9-_ ]{1,4086}.[a-zA-Z0-9-_ ]{1,10}")) {
+                if (fileName.matches("[a-zA-Z0-9-_ ()]{1,4086}.[a-zA-Z0-9-_ ]{1,10}")) { // "(" and ")" for duplicates files
                     wrongFile = false;
                 }
+            } else if (fileName.matches("[a-zA-Z0-9-_ ]{1,4086}.[a-zA-Z0-9-_ ]{1,10}")) {
+                wrongFile = false;
             }
         }
     }
@@ -320,13 +305,8 @@ public class SecuredUpload {
         boolean safeState = false;
         boolean fallbackOnApacheCommonsImaging;
 
-<<<<<<< HEAD
         if ((file != null) && file.exists() && file.canRead() && file.canWrite()) {
             try (OutputStream fos = Files.newOutputStream(file.toPath(), StandardOpenOption.WRITE)) {
-=======
-            if ((file != null) && file.exists() && file.canRead() && file.canWrite()) {
-                try (OutputStream fos = Files.newOutputStream(file.toPath(), StandardOpenOption.WRITE)) {
->>>>>>> 668e801ada (Improved: Secure the uploads (OFBIZ-12080))
                 // Get the image format
                 String formatName;
                 ImageInputStream iis = ImageIO.createImageInputStream(file);
@@ -403,16 +383,10 @@ public class SecuredUpload {
                 }
                 // Set state flag
                 safeState = true;
-<<<<<<< HEAD
             } catch (IOException | ImageReadException | ImageWriteException e) {
                 Debug.logWarning(e, "Error during Image file " + fileName + " processing !", MODULE);
-            }
-=======
-        } catch (IOException | ImageReadException | ImageWriteException e) {
-            Debug.logWarning(e, "Error during Image file " + fileName + " processing !", MODULE);
->>>>>>> 668e801ada (Improved: Secure the uploads (OFBIZ-12080))
         }
-    }
+        }
         return safeState;
     }