Document XML parser usage against security false positives

Prior to this commit, our XML parser usage would be already haredened
against XXE (XML External Entities) attacks. Still, we recently received
several invalid security reports claiming that our setup should be
hardened.

This commit documents a few usages of XML parsers to add some more
context and hopefully prevent future invalid reports.

Closes gh-33713
This commit is contained in:
Brian Clozel 2024-10-15 18:59:02 +02:00
parent 166714c8f5
commit f204f4962d
6 changed files with 20 additions and 2 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2018 the original author or authors. * Copyright 2002-2024 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -88,6 +88,9 @@ public class DefaultDocumentLoader implements DocumentLoader {
protected DocumentBuilderFactory createDocumentBuilderFactory(int validationMode, boolean namespaceAware) protected DocumentBuilderFactory createDocumentBuilderFactory(int validationMode, boolean namespaceAware)
throws ParserConfigurationException { throws ParserConfigurationException {
// This document loader is used for loading application configuration files.
// As a result, attackers would need complete write access to application configuration
// to leverage XXE attacks. This does not qualify as privilege escalation.
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setNamespaceAware(namespaceAware); factory.setNamespaceAware(namespaceAware);

View File

@ -157,6 +157,9 @@ final class PersistenceUnitReader {
Document buildDocument(ErrorHandler handler, InputStream stream) Document buildDocument(ErrorHandler handler, InputStream stream)
throws ParserConfigurationException, SAXException, IOException { throws ParserConfigurationException, SAXException, IOException {
// This document loader is used for loading application configuration files.
// As a result, attackers would need complete write access to application configuration
// to leverage XXE attacks. This does not qualify as privilege escalation.
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true); dbf.setNamespaceAware(true);
DocumentBuilder parser = dbf.newDocumentBuilder(); DocumentBuilder parser = dbf.newDocumentBuilder();

View File

@ -603,6 +603,9 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
Assert.hasLength(schemaLanguage, "No schema language provided"); Assert.hasLength(schemaLanguage, "No schema language provided");
Source[] schemaSources = new Source[resources.length]; Source[] schemaSources = new Source[resources.length];
// This parser is used to read the schema resources provided by the application.
// The parser used for reading the source is protected against XXE attacks.
// See "processSource(Source source)".
SAXParserFactory saxParserFactory = this.schemaParserFactory; SAXParserFactory saxParserFactory = this.schemaParserFactory;
if (saxParserFactory == null) { if (saxParserFactory == null) {
saxParserFactory = SAXParserFactory.newInstance(); saxParserFactory = SAXParserFactory.newInstance();
@ -907,6 +910,8 @@ public class Jaxb2Marshaller implements MimeMarshaller, MimeUnmarshaller, Generi
} }
try { try {
// By default, Spring will prevent the processing of external entities.
// This is a mitigation against XXE attacks.
if (xmlReader == null) { if (xmlReader == null) {
SAXParserFactory saxParserFactory = this.sourceParserFactory; SAXParserFactory saxParserFactory = this.sourceParserFactory;
if (saxParserFactory == null) { if (saxParserFactory == null) {

View File

@ -164,6 +164,8 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa
if (source instanceof StreamSource streamSource) { if (source instanceof StreamSource streamSource) {
InputSource inputSource = new InputSource(streamSource.getInputStream()); InputSource inputSource = new InputSource(streamSource.getInputStream());
try { try {
// By default, Spring will prevent the processing of external entities.
// This is a mitigation against XXE attacks.
SAXParserFactory saxParserFactory = this.sourceParserFactory; SAXParserFactory saxParserFactory = this.sourceParserFactory;
if (saxParserFactory == null) { if (saxParserFactory == null) {
saxParserFactory = SAXParserFactory.newInstance(); saxParserFactory = SAXParserFactory.newInstance();

View File

@ -177,6 +177,8 @@ public class SourceHttpMessageConverter<T extends Source> extends AbstractHttpMe
private DOMSource readDOMSource(InputStream body, HttpInputMessage inputMessage) throws IOException { private DOMSource readDOMSource(InputStream body, HttpInputMessage inputMessage) throws IOException {
try { try {
// By default, Spring will prevent the processing of external entities.
// This is a mitigation against XXE attacks.
DocumentBuilderFactory builderFactory = this.documentBuilderFactory; DocumentBuilderFactory builderFactory = this.documentBuilderFactory;
if (builderFactory == null) { if (builderFactory == null) {
builderFactory = DocumentBuilderFactory.newInstance(); builderFactory = DocumentBuilderFactory.newInstance();

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2021 the original author or authors. * Copyright 2002-2024 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -215,6 +215,9 @@ public class XsltView extends AbstractUrlBasedView {
} }
} }
else { else {
// This transformer is used for local XSLT views only.
// As a result, attackers would need complete write access to application configuration
// to leverage XXE attacks. This does not qualify as privilege escalation.
return TransformerFactory.newInstance(); return TransformerFactory.newInstance();
} }
} }