[ofbiz-framework] branch release22.01 updated: Fixed: Secure the uploads (OFBIZ-12080)

[jleroux]

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch release22.01
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


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

commit e9da7c0e4efcea8f03cd5d0d02aa5f86aa5d7b6d
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Sat Jun 11 11:09:26 2022 +0200

    Fixed: Secure the uploads (OFBIZ-12080)
    
    Several changes in SecuredUpload class:
    Mostly checks that uploaded CSV files are valid text files
    Also few debug log texts changes for clarity
---
 .../main/java/org/apache/ofbiz/security/SecuredUpload.java | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 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 29f02731c1..d86e5c594c 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
@@ -455,7 +455,7 @@ public class SecuredUpload {
             }
         } catch (Exception e) {
             safeState = false;
-            Debug.logError(e, "for security reason the PDF file " + 
file.getAbsolutePath() + "can't be uploaded !", MODULE);
+            Debug.logError(e, "for security reason the PDF file " + 
file.getAbsolutePath() + " can't be uploaded !", MODULE);
         }
         return safeState;
     }
@@ -493,14 +493,14 @@ public class SecuredUpload {
         try (CSVParser parser = new CSVParser(in, cvsFormat)) {
             parser.getRecords();
         }
-        return true;
+        return isValidTextFile(fileName, false); // Validate content to 
prevent webshell
     }
 
     private static boolean isExecutable(String fileName) throws IOException {
         String mimeType = getMimeTypeFromFileName(fileName);
         // Check for Windows executable. Neglect .bat and .ps1: 
https://s.apache.org/c8sim
         if ("application/x-msdownload".equals(mimeType) || 
"application/x-ms-installer".equals(mimeType)) {
-            Debug.logError("The file" + fileName + " is a Windows executable, 
for security reason it's not accepted :", MODULE);
+            Debug.logError("The file" + fileName + " is a Windows executable, 
for security reason it's not accepted", MODULE);
             return true;
         }
         // Check for ELF (Linux) and scripts
@@ -509,7 +509,7 @@ public class SecuredUpload {
                 || "application/text/x-perl".equals(mimeType)
                 || "application/text/x-ruby".equals(mimeType)
                 || "application/text/x-python".equals(mimeType)) {
-            Debug.logError("The file" + fileName + " is a Linux executable, 
for security reason it's not accepted :", MODULE);
+            Debug.logError("The file" + fileName + " is a Linux executable, 
for security reason it's not accepted", MODULE);
             return true;
         }
         return false;
@@ -661,7 +661,7 @@ public class SecuredUpload {
                 || "audio/x-flac".equals(mimeType)) {
             return true;
         }
-        Debug.logInfo("The file" + fileName + " is not a valid audio file, for 
security reason it's not accepted :", MODULE);
+        Debug.logInfo("The file" + fileName + " is not a valid audio file. For 
security reason it's not accepted as a such file", MODULE);
         return false;
     }
 
@@ -686,7 +686,7 @@ public class SecuredUpload {
                 || "video/x-ms-wmx".equals(mimeType)) {
             return true;
         }
-        Debug.logInfo("The file" + fileName + " is not a valid video file, for 
security reason it's not accepted :", MODULE);
+        Debug.logInfo("The file" + fileName + " is not a valid video file. For 
security reason it's not accepted as a such file", MODULE);
         return false;
     }
 
@@ -720,7 +720,7 @@ public class SecuredUpload {
     private static boolean isValid(String content, String string, List<String> 
allowed) {
         boolean isOK = !content.toLowerCase().contains(string) || 
allowed.contains(string);
         if (!isOK) {
-            Debug.logInfo("############### BEWARE, Image contains " + string, 
MODULE);
+            Debug.logInfo("The uploaded file contains the string '" + string + 
"'. It can't be uploaded for security reason", MODULE);
         }
         return isOK;
     }