secured DocumentBuilderFactory against XML External Entity (XXE) attacks

This commit is contained in:
Axel Uhl committed 2025-01-14 14:32:09 +01:00
1 parent 23b15b5479
commit 8044fee27b
6 files changed
+57 -20

No files matched your search

@@ -31,7 +31,6 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import org.apache.http.HttpMessage;
@@ -60,6 +59,7 @@ import com.sap.sse.common.TimePoint;
import com.sap.sse.common.Util;
import com.sap.sse.common.impl.MillisecondsTimePoint;
import com.sap.sse.util.LaxRedirectStrategyForAllRedirectResponseCodes;
import com.sap.sse.util.XmlUtil;
public class ORCPublicCertificateDatabaseImpl implements ORCPublicCertificateDatabase {
private static final Logger logger = Logger.getLogger(ORCPublicCertificateDatabaseImpl.class.getName());
@@ -365,7 +365,7 @@ public class ORCPublicCertificateDatabaseImpl implements ORCPublicCertificateDat
logger.fine(()->"Searching for "+params+"...");
final HttpResponse processorResponse = client.execute(postRequest);
final InputStream content = processorResponse.getEntity().getContent();
final DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
final DocumentBuilder builder = XmlUtil.getSecureDocumentBuilderFactory().newDocumentBuilder();
final Document document = builder.parse(content);
final NodeList dataNodes = document.getElementsByTagName(ROOT_ELEMENT).item(0).getChildNodes();
for (int dataNodeIndex=0; dataNodeIndex<dataNodes.getLength(); dataNodeIndex++) {
@@ -399,7 +399,7 @@ public class ORCPublicCertificateDatabaseImpl implements ORCPublicCertificateDat
logger.fine(()->"Searching for "+params+"...");
final HttpResponse processorResponse = client.execute(getRequest);
final InputStream content = processorResponse.getEntity().getContent();
final DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
final DocumentBuilder builder = XmlUtil.getSecureDocumentBuilderFactory().newDocumentBuilder();
final Document document = builder.parse(content);
final NodeList dataNodes = document.getElementsByTagName(ROOT_ELEMENT).item(0).getChildNodes();
for (int dataNodeIndex=0; dataNodeIndex<dataNodes.getLength(); dataNodeIndex++) {
@@ -13,5 +13,6 @@ Require-Bundle: com.sap.sailing.server,
com.sap.sse.common
Export-Package: com.sap.sailing.kiworesultimport
Bundle-Activator: com.sap.sailing.kiworesultimport.impl.Activator
Import-Package: org.osgi.framework
Import-Package: com.sap.sse.util,
org.osgi.framework
Automatic-Module-Name: com.sap.sailing.kiworesultimport
@@ -3,7 +3,6 @@ package com.sap.sailing.kiworesultimport.impl;
import java.io.IOException;
import java.io.InputStream;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import org.w3c.dom.Document;
@@ -11,10 +10,11 @@ import org.xml.sax.SAXException;
import com.sap.sailing.kiworesultimport.ResultList;
import com.sap.sailing.kiworesultimport.ResultListParser;
import com.sap.sse.util.XmlUtil;
public class ResultListParserImpl implements ResultListParser {
private Document parseDocument(InputStream inputStream) throws SAXException, IOException, ParserConfigurationException {
return DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(inputStream);
return XmlUtil.getSecureDocumentBuilderFactory().newDocumentBuilder().parse(inputStream);
}
@Override
@@ -11,11 +11,12 @@ import org.xml.sax.SAXException;
import com.sap.sailing.kiworesultimport.StartReport;
import com.sap.sailing.kiworesultimport.StartReportParser;
import com.sap.sse.util.XmlUtil;
public class StartReportParserImpl implements StartReportParser {
private Document parseDocument(InputStream inputStream) throws SAXException, IOException, ParserConfigurationException {
return DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(inputStream);
final DocumentBuilderFactory dbf = XmlUtil.getSecureDocumentBuilderFactory();
return dbf.newDocumentBuilder().parse(inputStream);
}
@Override
@@ -24,5 +25,4 @@ public class StartReportParserImpl implements StartReportParser {
StartReport result = new StartReportImpl(doc.getElementsByTagName("star:startbericht").item(0), sourceName);
return result;
}
}
@@ -5,15 +5,12 @@ import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import javax.xml.XMLConstants;
import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBElement;
import javax.xml.bind.JAXBException;
import javax.xml.bind.Unmarshaller;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParserFactory;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
import com.sap.sailing.domain.common.BoatClassMasterdata;
@@ -29,6 +26,7 @@ import com.sap.sailing.xrr.schema.Race;
import com.sap.sailing.xrr.schema.RegattaResults;
import com.sap.sailing.xrr.schema.TRResult;
import com.sap.sailing.xrr.schema.Team;
import com.sap.sse.util.XmlUtil;
public class ParserImpl implements Parser {
@@ -51,14 +49,7 @@ public class ParserImpl implements Parser {
@Override
public RegattaResults parse() throws JAXBException, SAXException, ParserConfigurationException {
final SAXParserFactory spf = SAXParserFactory.newInstance();
// This to defend against XXE:
spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
spf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
spf.setXIncludeAware(false);
// Do unmarshall operation
javax.xml.transform.Source xmlSource = new javax.xml.transform.sax.SAXSource(spf.newSAXParser().getXMLReader(),
new InputSource(inputStream));
javax.xml.transform.Source xmlSource = XmlUtil.getXmlSourceForInputStream(inputStream);
JAXBContext jc = JAXBContext.newInstance(TRResult.class.getPackage().getName(), ParserImpl.class.getClassLoader());
Unmarshaller um = jc.createUnmarshaller();
@SuppressWarnings("unchecked")
@@ -0,0 +1,45 @@
package com.sap.sse.util;
import java.io.InputStream;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParserFactory;
import javax.xml.transform.Source;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
import org.xml.sax.SAXNotRecognizedException;
import org.xml.sax.SAXNotSupportedException;
public class XmlUtil {
/**
* Creates an XML {@link Source} from an {@link InputStream} and secures it against XML External Entity (XXE)
* attacks by disallowing doctype parsing and enforcing {@link XMLConstants#FEATURE_SECURE_PROCESSING secure
* processing} to avoid denial-of-service attacks.
*/
public static javax.xml.transform.Source getXmlSourceForInputStream(final InputStream is)
throws ParserConfigurationException, SAXNotRecognizedException, SAXNotSupportedException, SAXException {
final SAXParserFactory spf = SAXParserFactory.newInstance();
// This to defend against XXE:
spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
spf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
spf.setXIncludeAware(false);
javax.xml.transform.Source xmlSource = new javax.xml.transform.sax.SAXSource(spf.newSAXParser().getXMLReader(), new InputSource(is));
return xmlSource;
}
/**
* Obtains a secure {@link DocumentBuilderFactory} that is configured to disallow doctype processing and includes.
* This way, it avoids XML External Entity (XXE) attacks as well as resource depletion / denial-of-service.
*/
public static DocumentBuilderFactory getSecureDocumentBuilderFactory() throws ParserConfigurationException {
final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
final String DISALLOW_DOCTYPE_FEATURE = "http://apache.org/xml/features/disallow-doctype-decl";
dbf.setFeature(DISALLOW_DOCTYPE_FEATURE, true);
dbf.setXIncludeAware(false);
return dbf;
}
}