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

[jl...@apache.org]

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

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


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

commit bc0363f12dfe4b73b17e9c66f52df05a4d70021b
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Thu Feb 17 18:48:34 2022 +0100

    Fixed: Secure the uploads (OFBIZ-12080)
    
    Prevents
    * too long lines (10 000) by default
    * linked images inside SVG
    
    Adds a comment about double extensions not allowed
---
 framework/security/config/security.properties      |  4 ++++
 .../org/apache/ofbiz/security/SecuredUpload.java   | 27 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index 4f69a46..5fda3be 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -222,6 +222,10 @@ deniedWebShellTokens=freemarker,<script,javascript,<body,<form,<jsp:,<c:out,tagl
                      python,perl ,/perl,ruby ,/ruby,process,function,class,InputStream,to_server,\
                      ifconfig,route,crontab,netstat,uname,hostname,iptables,whoami
 
+
+#-- Max line length for uploaded files, by default 10000
+maxLineLength=
+
 #-- Popup last-visited time from database after user has logged in.
 #-- So users can know of any unauthorised access to their accounts.
 #-- Default is false.
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 3d45685..d56482b 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
@@ -101,6 +101,7 @@ public class SecuredUpload {
     private static final String MODULE = SecuredUpload.class.getName();
     private static final List<String> DENIEDFILEEXTENSIONS = deniedFileExtensions();
     private static final List<String> DENIEDWEBSHELLTOKENS = deniedWebShellTokens();
+    private static final Integer maxLineLength = UtilProperties.getPropertyAsInteger("security", "maxLineLength", 10000);
 
     public static boolean isValidText(String content, List<String> allowed) throws IOException {
         return DENIEDWEBSHELLTOKENS.stream().allMatch(token -> isValid(content, token, allowed));
@@ -114,6 +115,13 @@ public class SecuredUpload {
 
         // Prevent double extensions
         if (StringUtils.countMatches(fileToCheck, ".") > 1) {
+            Debug.logError("Double extensions are not allowed for security reason", MODULE);
+            return false;
+        }
+
+        // Check max line length, default 10000
+        if (!checkMaxLinesLength(fileToCheck)) {
+            Debug.logError("For security reason lines over " + maxLineLength.toString() + " are not allowed", MODULE);
             return false;
         }
 
@@ -649,6 +657,10 @@ public class SecuredUpload {
         }
         }
         String content = new String(bytesFromFile);
+        if (content.toLowerCase().contains("xlink:href")) {
+            Debug.logError("Linked images inside SVG are not allowed for security reason", MODULE);
+            return false;
+        }
         ArrayList<String> allowed = new ArrayList<>();
         return isValidText(content, allowed);
     }
@@ -674,4 +686,19 @@ public class SecuredUpload {
         String deniedTokens = UtilProperties.getPropertyValue("security", "deniedWebShellTokens");
         return UtilValidate.isNotEmpty(deniedTokens) ? StringUtil.split(deniedTokens, ",") : new ArrayList<>();
     }
+
+    private static boolean checkMaxLinesLength(String fileToCheck) {
+        try {
+            File file = new File(fileToCheck);
+            List<String> lines = FileUtils.readLines(file, Charset.defaultCharset());
+            for (String line : lines) {
+                if (line.length() > maxLineLength) {
+                    return false;
+                }
+            }
+        } catch (IOException e) {
+            return false;
+        }
+        return true;
+    }
 }