Merge pull request #725 from Faerballert/feature/xml_parser_xxe_attacks

Unable to perform XML-oriented attacks
This commit is contained in:
Jochen Staerk
2025-02-19 15:50:02 +01:00
committed by GitHub
5 changed files with 41 additions and 10 deletions

View File

@@ -1,5 +1,6 @@
package org.mustangproject.ZUGFeRD; package org.mustangproject.ZUGFeRD;
import javax.xml.XMLConstants;
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
import org.apache.pdfbox.Loader; import org.apache.pdfbox.Loader;
import org.apache.pdfbox.pdmodel.PDDocument; import org.apache.pdfbox.pdmodel.PDDocument;
@@ -258,9 +259,14 @@ public class ZUGFeRDInvoiceImporter {
} }
private void setDocument() throws ParserConfigurationException, IOException, SAXException, ParseException { private void setDocument() throws ParserConfigurationException, IOException, SAXException, ParseException {
final DocumentBuilderFactory xmlFact = DocumentBuilderFactory.newInstance(); final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
xmlFact.setNamespaceAware(true); dbf.setNamespaceAware(true);
final DocumentBuilder builder = xmlFact.newDocumentBuilder(); dbf.setExpandEntityReferences(false);
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
final DocumentBuilder builder = dbf.newDocumentBuilder();
final ByteArrayInputStream is = new ByteArrayInputStream(rawXML); final ByteArrayInputStream is = new ByteArrayInputStream(rawXML);
/// is.skip(guessBOMSize(is)); /// is.skip(guessBOMSize(is));
document = builder.parse(is); document = builder.parse(is);

View File

@@ -21,6 +21,8 @@
package org.mustangproject.ZUGFeRD; package org.mustangproject.ZUGFeRD;
import com.helger.commons.io.stream.StreamHelper; import com.helger.commons.io.stream.StreamHelper;
import javax.xml.XMLConstants;
import javax.xml.parsers.ParserConfigurationException;
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
import org.apache.fop.apps.*; import org.apache.fop.apps.*;
import org.apache.fop.apps.io.ResourceResolverFactory; import org.apache.fop.apps.io.ResourceResolverFactory;
@@ -90,7 +92,8 @@ public class ZUGFeRDVisualizer {
* @param fis inputstream (will be consumed) * @param fis inputstream (will be consumed)
* @return (facturx = cii) * @return (facturx = cii)
*/ */
private EStandard findOutStandardFromRootNode(InputStream fis) { private EStandard findOutStandardFromRootNode(InputStream fis)
throws ParserConfigurationException {
String zf1Signature = "CrossIndustryDocument"; String zf1Signature = "CrossIndustryDocument";
String zf2Signature = "CrossIndustryInvoice"; String zf2Signature = "CrossIndustryInvoice";
@@ -100,6 +103,11 @@ public class ZUGFeRDVisualizer {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true); dbf.setNamespaceAware(true);
dbf.setExpandEntityReferences(false);
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
try { try {
DocumentBuilder db = dbf.newDocumentBuilder(); DocumentBuilder db = dbf.newDocumentBuilder();
Document doc = db.parse(new InputSource(fis)); Document doc = db.parse(new InputSource(fis));
@@ -121,12 +129,14 @@ public class ZUGFeRDVisualizer {
return null; return null;
} }
public String visualize(String xmlFilename, Language lang) throws IOException, TransformerException { public String visualize(String xmlFilename, Language lang)
throws IOException, TransformerException, ParserConfigurationException {
FileInputStream fis = new FileInputStream(xmlFilename); FileInputStream fis = new FileInputStream(xmlFilename);
return visualize(fis, lang); return visualize(fis, lang);
} }
public String visualize(InputStream inputXml, Language lang) throws IOException, TransformerException { public String visualize(InputStream inputXml, Language lang)
throws IOException, TransformerException, ParserConfigurationException {
initTemplates(lang); initTemplates(lang);
String fileContent = new String(IOUtils.toByteArray(inputXml), StandardCharsets.UTF_8); String fileContent = new String(IOUtils.toByteArray(inputXml), StandardCharsets.UTF_8);
@@ -211,7 +221,7 @@ public class ZUGFeRDVisualizer {
} }
protected String toFOP(String xmlFilename) protected String toFOP(String xmlFilename)
throws IOException, TransformerException { throws IOException, TransformerException, ParserConfigurationException {
FileInputStream fis = new FileInputStream(xmlFilename); FileInputStream fis = new FileInputStream(xmlFilename);
EStandard theStandard = findOutStandardFromRootNode(fis); EStandard theStandard = findOutStandardFromRootNode(fis);
@@ -264,7 +274,7 @@ public class ZUGFeRDVisualizer {
*/ */
try { try {
fopInput = this.toFOP(XMLinputFile.getAbsolutePath()); fopInput = this.toFOP(XMLinputFile.getAbsolutePath());
} catch (TransformerException | IOException e) { } catch (TransformerException | IOException | ParserConfigurationException e) {
LOGGER.error("Failed to apply FOP", e); LOGGER.error("Failed to apply FOP", e);
} }
@@ -291,7 +301,7 @@ public class ZUGFeRDVisualizer {
fis = new ByteArrayInputStream(xmlContent.getBytes(StandardCharsets.UTF_8));//rewind :-( fis = new ByteArrayInputStream(xmlContent.getBytes(StandardCharsets.UTF_8));//rewind :-(
fopInput = toFOP(fis, theStandard); fopInput = toFOP(fis, theStandard);
} catch (TransformerException | IOException e) { } catch (TransformerException | IOException | ParserConfigurationException e) {
LOGGER.error("Failed to apply FOP", e); LOGGER.error("Failed to apply FOP", e);
} }

View File

@@ -20,6 +20,7 @@
*/ */
package org.mustangproject.ZUGFeRD; package org.mustangproject.ZUGFeRD;
import javax.xml.parsers.ParserConfigurationException;
import org.junit.FixMethodOrder; import org.junit.FixMethodOrder;
import org.junit.runners.MethodSorters; import org.junit.runners.MethodSorters;
import org.mustangproject.ZUGFeRD.ZUGFeRDVisualizer.Language; import org.mustangproject.ZUGFeRD.ZUGFeRDVisualizer.Language;
@@ -76,9 +77,10 @@ public class VisualizationTest extends ResourceCase {
fail("TransformerException should not happen: " + e.getMessage()); fail("TransformerException should not happen: " + e.getMessage());
} catch (IOException e) { } catch (IOException e) {
fail("IOException should not happen: " + e.getMessage()); fail("IOException should not happen: " + e.getMessage());
} catch (ParserConfigurationException e) {
fail("ParserConfigurationException should not happen: " + e.getMessage());
} }
assertNotNull(result); assertNotNull(result);
/* remove file endings so that tests can also pass after checking /* remove file endings so that tests can also pass after checking
out from git with arbitrary options (which may include CSRF changes) out from git with arbitrary options (which may include CSRF changes)

View File

@@ -10,6 +10,7 @@ import java.nio.file.Files;
import java.nio.file.Paths; import java.nio.file.Paths;
import java.util.Calendar; import java.util.Calendar;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.transform.stream.StreamSource; import javax.xml.transform.stream.StreamSource;
@@ -151,6 +152,11 @@ public class XMLValidator extends Validator {
final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true); // otherwise we can not act namespace independently, i.e. use dbf.setNamespaceAware(true); // otherwise we can not act namespace independently, i.e. use
// document.getElementsByTagNameNS("*",... // document.getElementsByTagNameNS("*",...
dbf.setExpandEntityReferences(false);
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
final DocumentBuilder db = dbf.newDocumentBuilder(); final DocumentBuilder db = dbf.newDocumentBuilder();
final InputSource is = new InputSource(new StringReader(zfXML)); final InputSource is = new InputSource(new StringReader(zfXML));

View File

@@ -17,6 +17,7 @@ import java.text.SimpleDateFormat;
import java.util.Calendar; import java.util.Calendar;
import java.util.Date; import java.util.Date;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.DocumentBuilderFactory;
@@ -142,6 +143,12 @@ public class ZUGFeRDValidator {
String xmlAsString = null; String xmlAsString = null;
try { try {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true);
dbf.setExpandEntityReferences(false);
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
DocumentBuilder db = dbf.newDocumentBuilder(); DocumentBuilder db = dbf.newDocumentBuilder();
content = XMLTools.removeBOM(content); content = XMLTools.removeBOM(content);