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 2024/03/12 09:32:12 UTC
(ofbiz-framework) 02/02: Fixed: SecuredUpload::checkMaxLinesLength put in a regression for PDF files (OFBIZ-12936)
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 773c2b2455bcf887ae22efcafce2f619955834e0
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Tue Mar 12 10:26:59 2024 +0100
Fixed: SecuredUpload::checkMaxLinesLength put in a regression for PDF files (OFBIZ-12936)
As commented at https://s.apache.org/wpkuj, while working on OFBIZ-12926
I realised that checkMaxLinesLength put in a possible regression for PDF files.
As a summary: we can't use checkMaxLinesLength for PDF files because they may
contain several CharSet encodings. Hence no possibility to use
Files::readAllLines that needs a sole CharSet. So we need to bypass this check.
---
.../org/apache/ofbiz/security/SecuredUpload.java | 30 +++++++++++++++++++---
1 file changed, 26 insertions(+), 4 deletions(-)
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 f1510da085..8ab076d656 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
@@ -202,10 +202,14 @@ public class SecuredUpload {
// Check the file content
- // Check max line length, default 10000
- if (!checkMaxLinesLength(fileToCheck)) {
- Debug.logError("For security reason lines over " + MAXLINELENGTH.toString() + " are not allowed", MODULE);
- return false;
+ // Check max line length, default 10000.
+ // PDF files are not concerned because they may contain several CharSet encodings
+ // hence no possibility to use Files::readAllLines that needs a sole CharSet
+ if (!isPdfFile(fileToCheck)) {
+ if (!checkMaxLinesLength(fileToCheck)) {
+ Debug.logError("For security reason lines over " + MAXLINELENGTH.toString() + " are not allowed", MODULE);
+ return false;
+ }
}
if (isExecutable(fileToCheck)) {
@@ -433,6 +437,24 @@ public class SecuredUpload {
return false;
}
+ /**
+ * @param fileName
+ * @return true if it's a PDF file
+ */
+ private static boolean isPdfFile(String fileName) {
+ File file = new File(fileName);
+ try {
+ if (Objects.isNull(file) || !file.exists()) {
+ return false;
+ }
+ // Load stream in PDF parser
+ PdfReader reader = new PdfReader(file.getAbsolutePath());
+ return true;
+ } catch (Exception e) {
+ return false;
+ }
+ }
+
/**
* @param fileName
* @return true if it's a safe PDF file: is PDF and does not contains embedded files