Improved: Allow configuration of file name validation pattern (OFBIZ-…

…12851)

Read file name validation pattern from security.properties to allow customization
Explanation:
Hard coding the pattern made it difficult to adjust file name validation.

jleroux: Rather than pushing the PR, which is OK with me, I apply as a patch
locally and make some modifications before pushing:
indentation in SecuredUpload, and warning about file names safeness in
security.properties

Thanks: originalnichtskoenner for this PR on GH:
#668.

framework/security/config/security.properties

@@ -247,6 +247,13 @@ deniedFileExtensions=html,htm,php,php1,php2,hph3,php4,php5,php6,php7,phps,asp,as
 #-- As it name says, allowAllUploads opens all possibilities
 allowAllUploads=
 
+#--
+#-- Default characters that are allowed in file names and file extensions to guarantee safeness
+#-- Uncomment to change. Note that allowing all characters is at risk.
+#-- You may refer to https://cheatsheetseries.owasp.org/cheatsheets/File_Upload_Cheat_Sheet.html#filename-sanitization
+#fileNameValidCharacters=[a-zA-Z0-9-_ ]
+#fileNameValidCharactersDuplicates=[a-zA-Z0-9-_ ()]
+
 #--
 #-- CSV format used to upload CSV files, cf. https://commons.apache.org/proper/commons-csv/apidocs/org/apache/commons/csv/CSVFormat.html
 csvformat=CSVFormat.DEFAULT

framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java

@@ -108,6 +108,12 @@ public class SecuredUpload {
     private static final Boolean ALLOWSTRINGCONCATENATIONINUPLOADEDFILES =
             UtilProperties.getPropertyAsBoolean("security", "allowStringConcatenationInUploadedFiles", false);
 
+    // "(" and ")" for duplicates files
+    private static final String FILENAMEVALIDCHARACTERS_DUPLICATES =
+            UtilProperties.getPropertyValue("security", "fileNameValidCharactersDuplicates", "[a-zA-Z0-9-_ ()]");
+    private static final String FILENAMEVALIDCHARACTERS =
+            UtilProperties.getPropertyValue("security", "fileNameValidCharacters", "[a-zA-Z0-9-_ ]");
+
     public static boolean isValidText(String content, List<String> allowed) throws IOException {
         String contentWithoutSpaces = content.replace(" ", "");
         if ((contentWithoutSpaces.contains("\"+\"") || contentWithoutSpaces.contains("'+'"))
@@ -157,21 +163,37 @@ public static boolean isValidFileName(String fileToCheck, Delegator delegator) t
                     Debug.logError("Uploaded file name too long", MODULE);
                 } else if (p.toString().contains(imageServerUrl.replace("/", "\\"))) {
                     // 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
+                    if (fileName.matches(
+                            FILENAMEVALIDCHARACTERS_DUPLICATES
+                            .concat("{1,249}.")
+                            .concat(FILENAMEVALIDCHARACTERS)
+                            .concat("{1,10}"))) {
                         wrongFile = false;
                     }
-                } else if (fileName.matches("[a-zA-Z0-9-_ ]{1,249}.[a-zA-Z0-9-_ ]{1,10}")) {
+                } else if (fileName.matches(
+                        FILENAMEVALIDCHARACTERS
+                        .concat("{1,249}.")
+                        .concat(FILENAMEVALIDCHARACTERS)
+                        .concat("{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
+                    if (fileName.matches(
+                            FILENAMEVALIDCHARACTERS_DUPLICATES
+                            .concat("{1,4086}.")
+                            .concat(FILENAMEVALIDCHARACTERS)
+                            .concat("{1,10}"))) {
                         wrongFile = false;
                     }
-                } else if (fileName.matches("[a-zA-Z0-9-_ ]{1,4086}.[a-zA-Z0-9-_ ]{1,10}")) {
+                } else if (fileName.matches(
+                        FILENAMEVALIDCHARACTERS
+                        .concat("{1,4086}.")
+                        .concat(FILENAMEVALIDCHARACTERS)
+                        .concat("{1,10}"))) {
                     wrongFile = false;
                 }
             }