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 8c2d759847a7b22cb355a2621eb520d043591e4f
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Fri Feb 4 04:30:13 2022 +0100
Fixed: Remote Code Execution (File Upload) Vulnerability (OFBIZ-11948)
Lion Tree <liontree0...@gmail.com> has reported us that
"CVE-2020-1938 is not fully fixed".
Though it was fixed by OFBIZ-11407, it still possible for an authenticated
user
to upload a webshell included in an image using one of the upload
possibilities
in OFBiz. That is not new and covered by OFBIZ-12080 "Secure the uploads",
but
was still incomplete.
This enforces the secured uploads by
* checking in SecuredUpload::isValidImageFile that a webshell is not
embedded in
an image.
* Keeping only "<%" as a denied token for JSP webshells, instead of
currently
"<%@ page"
* Adds "application/text/x-ruby" to SecuredUpload::isExecutable
Also
* Adds "<jsp", and "<?" for PHP. Even if OFBiz does not use PHP at all,
it's often installed on servers.
* Removes "import=\"java" and "runtime.getruntime().exec(". They are no
longer useful since "<%" and "<jsp" block them.
* Remove php token since I'll put "<?" in.
* Adds "#!", rather than adding other shebangs like perl,python and ruby
This will make deniedWebShellTokens more understandable.
But I'm conscious that despite SecuredUpload::isExecutableI I still need to
better handle encoded webshells. I'll do that soon in a second approach.
I'll also certainly more prune PHP related tokens.
Thanks: Lion Tree for report
---
framework/security/config/security.properties | 6 +++---
.../src/main/java/org/apache/ofbiz/security/SecuredUpload.java | 4 +++-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index 5816f0c..9eccffb 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -238,9 +238,9 @@ allowAllUploads=
#-- eg:
https://www.acunetix.com/blog/articles/detection-prevention-introduction-web-shells-part-5/
#-- "freemarker" should be OK, should not be used in Freemarker templates, not
part of the syntax.
#-- Else "template.utility.Execute" is a good replacement but not as much
catching, who knows...
-deniedWebShellTokens=freemarker,import=\"java,runtime.getruntime().exec(,<%@
page,<script,<body>,<form,php,\
- javascript,%eval,@eval,import
os,passthru,exec,shell_exec,assert,str_rot13,system,phpinfo,base64_decode,chmod,mkdir,\
- fopen,fclose,new
file,import,upload,getfilename,download,getoutputstring,readfile
+deniedWebShellTokens=<%,<jsp:,<?,#!,freemarker,<script,javascript,%eval,@eval,<body>,<form,\
+ import
os,passthru,exec,shell_exec,assert,str_rot13,system,base64_decode,chmod,mkdir,\
+ fopen,fclose,new
file,import,upload,getfilename,download,getoutputstring,readfile
#-- Popup last-visited time from database after user has logged in.
#-- So users can know of any unauthorised access to their accounts.
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 038733d..0f09496 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
@@ -244,7 +244,8 @@ public class SecuredUpload {
|| imageFormat.equals(ImageFormats.GIF)
|| imageFormat.equals(ImageFormats.TIFF)
|| imageFormat.equals(ImageFormats.JPEG))
- && imageMadeSafe(fileName);
+ && imageMadeSafe(fileName)
+ && isValidTextFile(fileName);
}
/**
@@ -418,6 +419,7 @@ public class SecuredUpload {
if ("application/x-elf".equals(mimeType)
|| "application/x-sh".equals(mimeType)
|| "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);
return true;
