[ofbiz-framework] branch trunk 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 trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 756b5d2  Fixed: Secure the uploads (OFBIZ-12080)
756b5d2 is described below

commit 756b5d237ef45a6c90e1b58b710e04996f8f5c7a
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 e2c1473..7661561 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -255,6 +255,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 f595f80..f592091 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;
         }
 
@@ -646,6 +654,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);
     }
@@ -671,4 +683,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;
+    }
 }