[ofbiz-framework] 01/02: Fixed: Disallow string concatenation in uploaded files (OFBIZ-12794)

[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

commit 6477a8929e1a7f535818d277c082024c4f2e6575
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Sat Apr 8 11:06:02 2023 +0200

    Fixed: Disallow string concatenation in uploaded files (OFBIZ-12794)
    
    An external security reporter brought to our attention that a signed up user
    could upload a webshell using string concatenation. Of course there is no 
reason
    for a signed up user to upload a webshell. And anyway we don't create CVEs 
for
    signed up users trying our security.
    
    Nevertheless we have decided to fix this possibility while allowing to 
bypass it
    using a new security property. The later can be useful when a file must 
contain
    a string concatenation, images files, seen as encoded texts, come to mind.
    
    Thanks: so far unknown security reporter
---
 framework/security/config/security.properties                      | 2 ++
 .../src/main/java/org/apache/ofbiz/security/SecuredUpload.java     | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/framework/security/config/security.properties 
b/framework/security/config/security.properties
index 1e79caa265..44119dc361 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -270,6 +270,8 @@ 
deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,body ,<form
                      python,perl ,/perl,ruby 
,/ruby,process,function,class,InputStream,to_server,wget 
,static,assign,webappPath,\
                      ifconfig,route,crontab,netstat,uname 
,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost,thread,require,gzdeflate
 
+allowStringConcatenationInUploadedFiles=false
+
 #-- Max line length for uploaded files, by default 10000
 maxLineLength=
 
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 fb9fd68ed3..1b0a851761 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
@@ -105,8 +105,15 @@ public class SecuredUpload {
     private static final List<String> DENIEDFILEEXTENSIONS = 
getDeniedFileExtensions();
     private static final List<String> DENIEDWEBSHELLTOKENS = 
getDeniedWebShellTokens();
     private static final Integer MAXLINELENGTH = 
UtilProperties.getPropertyAsInteger("security", "maxLineLength", 10000);
+    private static final Boolean allowStringConcatenationInUploadedFiles = 
UtilProperties.getPropertyAsBoolean("security", 
"allowStringConcatenationInUploadedFiles", false);
 
     public static boolean isValidText(String content, List<String> allowed) 
throws IOException {
+        String contentWithoutSpaces = content.replace(" ", "");
+        if ((contentWithoutSpaces.contains("\"+\"") || 
contentWithoutSpaces.contains("'+'"))
+                && !allowStringConcatenationInUploadedFiles) {
+            Debug.logInfo("The uploaded file contains a string concatenation. 
It can't be uploaded for security reason", MODULE);
+            return false;
+        }
         return content != null ? DENIEDWEBSHELLTOKENS.stream().allMatch(token 
-> isValid(content, token.toLowerCase(), allowed)) : false;
     }