You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2022/02/04 11:31:50 UTC
[ofbiz-framework] 01/02: Fixed: Remote Code Execution (File Upload) Vulnerability (OFBIZ-11948)
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
commit e7955fc06438cfa3e93cfbf00291958fceb3d4ed
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Fri Feb 4 04:30:13 2022 +0100
Fixed: Remote Code Execution (File Upload) Vulnerability (OFBIZ-11948)
Lion Tree <li...@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 5861ef4..cda4b36 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -205,9 +205,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
#-- uri used for login (cf jira OFBIZ-12047)
#-- it's a list, each uri should be separated by comma, without space
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 afa2f7b..0cd9a70 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
@@ -242,7 +242,8 @@ public class SecuredUpload {
|| imageFormat.equals(ImageFormats.GIF)
|| imageFormat.equals(ImageFormats.TIFF)
|| imageFormat.equals(ImageFormats.JPEG))
- && imageMadeSafe(fileName);
+ && imageMadeSafe(fileName)
+ && isValidTextFile(fileName);
}
/**
@@ -422,6 +423,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;