[ofbiz-framework] 02/04: Improved: Secure the uploads (OFBIZ-12080)

[jleroux]

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 9b497d55cbf1f19a8280cbed5e581208a1d937eb
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Sat Feb 19 11:44:32 2022 +0100

    Improved: Secure the uploads (OFBIZ-12080)
    
    Manually merges SecuredUpload.java based on 18.12
---
 .../org/apache/ofbiz/security/SecuredUpload.java   | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 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 1d117bd..563d6cb 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
@@ -107,7 +107,7 @@ public class SecuredUpload {
         return DENIEDWEBSHELLTOKENS.stream().allMatch(token -> 
isValid(content, token, allowed));
     }
 
-    private static boolean isValidFileName(String fileToCheck, Delegator 
delegator) throws IOException {
+    public static boolean isValidFileName(String fileToCheck, Delegator 
delegator) throws IOException {
         // Allow all
         if 
(("true".equalsIgnoreCase(EntityUtilProperties.getPropertyValue("security", 
"allowAllUploads", delegator)))) {
             return true;
@@ -119,7 +119,6 @@ public class SecuredUpload {
             return false;
         }
 
-
         // Check max line length, default 10000
         if (!checkMaxLinesLength(fileToCheck)) {
             Debug.logError("For security reason lines over " + 
MAXLINELENGTH.toString() + " are not allowed", MODULE);
@@ -133,18 +132,19 @@ public class SecuredUpload {
         // Check extensions
         if (p != null && p.getFileName() != null) {
             String fileName = p.getFileName().toString(); // The file name is 
the farthest element from the root in the directory hierarchy.
+            String extension = 
FilenameUtils.getExtension(fileToCheck).toLowerCase();
             // Prevents null byte in filename
-            if (fileName.contains("%00")
-                    || fileName.contains("%0a")
-                    || fileName.contains("%20")
-                    || fileName.contains("%0d%0a")
-                    || fileName.contains("/")
-                    || fileName.contains("./")
-                    || fileName.contains(".")) {
-                Debug.logError("Special bytes in filename are not allowed for 
security reason", MODULE);
+            if (extension.contains("%00")
+                    || extension.contains("%0a")
+                    || extension.contains("%20")
+                    || extension.contains("%0d%0a")
+                    || extension.contains("/")
+                    || extension.contains("./")
+                    || extension.contains(".")) {
+                Debug.logError("Special bytes in extension are not allowed for 
security reason", MODULE);
                 return false;
             }
-            if 
(DENIEDFILEEXTENSIONS.contains(FilenameUtils.getExtension(fileToCheck).toLowerCase()))
 {
+            if (DENIEDFILEEXTENSIONS.contains(extension)) {
                 Debug.logError("This file extension is not allowed for 
security reason", MODULE);
                 deleteBadFile(fileToCheck);
                 return false;