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/06/11 07:09:09 UTC
[ofbiz-framework] branch release22.01 updated: Fixed: Unable to upload a file through ecommerce (OFBIZ-12636)
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
The following commit(s) were added to refs/heads/release22.01 by this push:
new 4f469fdfb7 Fixed: Unable to upload a file through ecommerce (OFBIZ-12636)
4f469fdfb7 is described below
commit 4f469fdfb70c7c918286cae9f4dd2695a8a2759d
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Sat Jun 11 08:57:36 2022 +0200
Fixed: Unable to upload a file through ecommerce (OFBIZ-12636)
We need to upload a CSV format file as part of project requirement
We were able upload CSV format file under eCommerce in Version 16.11.07.
We are trying to implement the same in version 18.12.05 - there the same upload
module does not work.
jleroux: I missed to check and allow CSV files when all files are allowed.
That now uses:
https://commons.apache.org/proper/commons-csv/apidocs/org/apache/commons/csv/CSVFormat.html
The default CSV format used to check upload of CSV files is set in the property
csvformat=CSVFormat.DEFAULT in security.properties
Thanks: Sachin for report
---
framework/security/config/security.properties | 6 +++
.../org/apache/ofbiz/security/SecuredUpload.java | 49 ++++++++++++++++++++--
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index 03dbc8ee6b..46a0280c7b 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -244,6 +244,11 @@ deniedFileExtensions=html,htm,php,php1,php2,hph3,php4,php5,php6,php7,phps,asp,as
#-- As it name says, allowAllUploads opens all possibilities
allowAllUploads=
+#--
+#-- CSV format used to upload CSV files, cf. https://commons.apache.org/proper/commons-csv/apidocs/org/apache/commons/csv/CSVFormat.html
+csvformat=CSVFormat.DEFAULT
+
+
#--
#-- List of denied tokens often part of webshells. Note that, for now at least, most are supposed to be used on a *nix system
#-- TODO.... to be continued with known webshell contents... a complete allow list is impossible anyway...
@@ -285,3 +290,4 @@ allowedProtocols=localhost,127.0.0.1
#-- Prevent Freemarker exploits
#-- eg: allowedURIsForFreemarkerInterpolation=createTextContentCms,updateTextContentCms,...
allowedURIsForFreemarkerInterpolation=
+
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 f2d92f4e31..29f02731c1 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
@@ -27,6 +27,8 @@ import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
import java.nio.ByteBuffer;
import java.nio.charset.CharacterCodingException;
import java.nio.charset.Charset;
@@ -49,6 +51,8 @@ import javax.imageio.stream.ImageInputStream;
import org.apache.batik.dom.svg.SAXSVGDocumentFactory;
import org.apache.batik.util.XMLResourceDescriptor;
+import org.apache.commons.csv.CSVFormat;
+import org.apache.commons.csv.CSVParser;
import org.apache.commons.imaging.ImageFormat;
import org.apache.commons.imaging.ImageFormats;
import org.apache.commons.imaging.ImageInfo;
@@ -192,7 +196,7 @@ public class SecuredUpload {
}
// Check the file name
- if (!isValidFileName(fileToCheck, delegator)) {
+ if (!isValidFileName(fileToCheck, delegator)) { // Useless when the file is internally generated, but not sure for all cases
return false;
}
@@ -268,6 +272,7 @@ public class SecuredUpload {
|| isValidCompressedFile(fileToCheck, delegator)
|| isValidAudioFile(fileToCheck)
|| isValidVideoFile(fileToCheck)
+ || isValidCsvFile(fileToCheck)
|| isValidPdfFile(fileToCheck)) {
return true;
}
@@ -455,6 +460,42 @@ public class SecuredUpload {
return safeState;
}
+ /**
+ * Is it a CVS file?
+ * @param fileName
+ * @return true if it's a valid CVS file
+ * @throws IOException
+ */
+ private static boolean isValidCsvFile(String fileName) throws IOException {
+ Path filePath = Paths.get(fileName);
+ String content = new String(Files.readAllBytes(filePath));
+ Reader in = new StringReader(content);
+ String cvsFormatString = UtilProperties.getPropertyValue("security", "csvformat");
+ CSVFormat cvsFormat = CSVFormat.DEFAULT;
+ switch (cvsFormatString) {
+ case "EXCEL":
+ cvsFormat = CSVFormat.EXCEL;
+ break;
+ case "MYSQL":
+ cvsFormat = CSVFormat.MYSQL;
+ break;
+ case "ORACLE":
+ cvsFormat = CSVFormat.ORACLE;
+ break;
+ case "POSTGRESQL_CSV":
+ cvsFormat = CSVFormat.POSTGRESQL_CSV;
+ break;
+ default:
+ cvsFormat = CSVFormat.DEFAULT;
+ }
+
+ // cf. https://commons.apache.org/proper/commons-csv/apidocs/org/apache/commons/csv/CSVFormat.html
+ try (CSVParser parser = new CSVParser(in, cvsFormat)) {
+ parser.getRecords();
+ }
+ return true;
+ }
+
private static boolean isExecutable(String fileName) throws IOException {
String mimeType = getMimeTypeFromFileName(fileName);
// Check for Windows executable. Neglect .bat and .ps1: https://s.apache.org/c8sim
@@ -620,7 +661,7 @@ public class SecuredUpload {
|| "audio/x-flac".equals(mimeType)) {
return true;
}
- Debug.logError("The file" + fileName + " is not a valid audio file, for security reason it's not accepted :", MODULE);
+ Debug.logInfo("The file" + fileName + " is not a valid audio file, for security reason it's not accepted :", MODULE);
return false;
}
@@ -645,7 +686,7 @@ public class SecuredUpload {
|| "video/x-ms-wmx".equals(mimeType)) {
return true;
}
- Debug.logError("The file" + fileName + " is not a valid video file, for security reason it's not accepted :", MODULE);
+ Debug.logInfo("The file" + fileName + " is not a valid video file, for security reason it's not accepted :", MODULE);
return false;
}
@@ -669,7 +710,7 @@ public class SecuredUpload {
String content = new String(bytesFromFile);
if (content.toLowerCase().contains("xlink:href=\"http")
|| content.toLowerCase().contains("<!ENTITY ")) { // Billions laugh attack
- Debug.logError("Linked images inside or Entity in SVG are not allowed for security reason", MODULE);
+ Debug.logInfo("Linked images inside or Entity in SVG are not allowed for security reason", MODULE);
return false;
}
ArrayList<String> allowed = new ArrayList<>();