This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new c3653d51ae Improved: Make sure no pdf files containing unwanted
attachments can be uploaded (OFBIZ-12926)
c3653d51ae is described below
commit c3653d51ae88e6a0069bb750d8d36c7fa0e4e1a7
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Tue Mar 12 10:47:52 2024 +0100
Improved: Make sure no pdf files containing unwanted attachments can be
uploaded
(OFBIZ-12926)
Puts in the allowZUGFeRDnotSecure as a "true" security property.
For now it allows to accept PDF files with only 1 XML attachment.
This is a near definitive configuration, still an improvement rather than a
fix
---
framework/security/config/security.properties | 3 ++
.../org/apache/ofbiz/security/SecuredUpload.java | 38 ++++++++++++++++++----
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index b4cedc4553..701d07afb3 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -287,6 +287,9 @@ maxLineLength=
# Allow uploading non-empty pdf files as long as they are ZUGFeRD compliant
allowZUGFeRDCompliantUpload=true
+# Allow uploading ZUGFeRD compliant files with no XML header and no schema
+allowZUGFeRDnotSecure=true
+
#-- Popup last-visited time from database after user has logged in.
#-- So users can know of any unauthorised access to their accounts.
#-- Default is false.
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 b61c780960..08bb5ab8e9 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
@@ -49,6 +49,7 @@ import java.util.UUID;
import javax.imageio.ImageIO;
import javax.imageio.ImageReader;
import javax.imageio.stream.ImageInputStream;
+import javax.xml.parsers.ParserConfigurationException;
import org.apache.batik.anim.dom.SAXSVGDocumentFactory;
import org.apache.batik.util.XMLResourceDescriptor;
@@ -72,6 +73,7 @@ import org.apache.ofbiz.base.util.FileUtil;
import org.apache.ofbiz.base.util.StringUtil;
import org.apache.ofbiz.base.util.UtilProperties;
import org.apache.ofbiz.base.util.UtilValidate;
+import org.apache.ofbiz.base.util.UtilXml;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.entity.util.EntityUtilProperties;
import org.apache.pdfbox.pdmodel.PDDocument;
@@ -89,6 +91,7 @@ import org.apache.tika.sax.BasicContentHandlerFactory;
import org.apache.tika.sax.ContentHandlerFactory;
import org.apache.tika.sax.RecursiveParserWrapperHandler;
import org.mustangproject.ZUGFeRD.ZUGFeRDImporter;
+import org.w3c.dom.Document;
import org.xml.sax.SAXException;
import com.lowagie.text.pdf.PdfReader;
@@ -488,12 +491,12 @@ public class SecuredUpload {
/**
* @param fileName
- * @return true if it's a safe PDF file: is PDF and does not contains
embedded files
+ * @return true if it's a safe PDF file: is a PDF, and contains only 1
embedded readable (valid and secure) XML file (zUGFeRD)
*/
private static boolean isValidPdfFile(String fileName) {
File file = new File(fileName);
boolean safeState = false;
- boolean canParse = false;
+ boolean canParseZUGFeRD = true;
try {
if (Objects.isNull(file) || !file.exists()) {
return safeState;
@@ -512,21 +515,42 @@ public class SecuredUpload {
PDDocumentNameDictionary names = new
PDDocumentNameDictionary(pdDocument.getDocumentCatalog());
efTree = names.getEmbeddedFiles();
}
- boolean zUGFeRDCompliantUploadAllowed =
UtilProperties.getPropertyAsBoolean(
- "security", "allowZUGFeRDCompliantUpload", false);
+ boolean zUGFeRDCompliantUploadAllowed =
UtilProperties.getPropertyAsBoolean("security", "allowZUGFeRDCompliantUpload",
false);
if (zUGFeRDCompliantUploadAllowed && !Objects.isNull(efTree)) {
+ canParseZUGFeRD = false;
Integer numberOfEmbeddedFiles = efTree.getNames().size();
if (numberOfEmbeddedFiles.equals(1)) {
ZUGFeRDImporter importer = new
ZUGFeRDImporter(file.getAbsolutePath());
- canParse = importer.canParse();
+ boolean allowZUGFeRDnotSecure =
UtilProperties.getPropertyAsBoolean("security", "allowZUGFeRDnotSecure", false);
+ if (allowZUGFeRDnotSecure) {
+ canParseZUGFeRD = importer.canParse();
+ } else {
+ try {
+ Document document =
UtilXml.readXmlDocument(importer.getUTF8());
+ if (document.toString().equals("[#document:
null]")) {
+ safeState = false;
+ Debug.logInfo("The file " +
file.getAbsolutePath()
+ + " is not a readable (valid and
secure) PDF file. For security reason it's not accepted as a such file",
+ MODULE);
+
+ }
+ } catch (SAXException | ParserConfigurationException |
IOException e) {
+ safeState = false;
+ Debug.logInfo(e, "The file " +
file.getAbsolutePath()
+ + " is not a readable (valid and secure)
PDF file. For security reason it's not accepted as a such file",
+ MODULE);
+ }
+ }
}
}
- safeState = Objects.isNull(efTree) || canParse;
+ safeState = Objects.isNull(efTree) || canParseZUGFeRD;
} catch (Exception e) {
safeState = false;
- Debug.logInfo(e, "The file " + file.getAbsolutePath() + " is not a
valid PDF file. For security reason it's not accepted as a such file",
+ Debug.logInfo(e, "The file " + file.getAbsolutePath() + " is not a
readable (valid and secure) PDF file. For security reason it's not accepted as
a such file",
MODULE);
}
+ file = new File(fileName);
+ file.delete();
return safeState;
}
