This is an automated email from the ASF dual-hosted git repository. Claudenw pushed a commit to branch fix-xxe-issues in repository https://gitbox.apache.org/repos/asf/creadur-rat.git
commit 7bddd0434b6bdaa88599b102f961d9b2c4376570 Author: Claude Warren <[email protected]> AuthorDate: Tue Jun 16 16:48:30 2026 +0100 changes to reduce XXE exposure --- .../src/main/java/org/apache/rat/Reporter.java | 6 ++--- .../rat/configuration/XMLConfigurationReader.java | 25 +++--------------- .../org/apache/rat/utils/StandardXmlFactory.java | 30 ++++++++++++++++++++++ .../org/apache/rat/ReporterOptionsProvider.java | 8 +++--- .../java/org/apache/rat/testhelpers/XmlUtils.java | 21 ++++++++------- .../java/org/apache/rat/anttasks/ReportTest.java | 4 +-- 6 files changed, 55 insertions(+), 39 deletions(-) diff --git a/apache-rat-core/src/main/java/org/apache/rat/Reporter.java b/apache-rat-core/src/main/java/org/apache/rat/Reporter.java index 2e58c582..d387dd44 100644 --- a/apache-rat-core/src/main/java/org/apache/rat/Reporter.java +++ b/apache-rat-core/src/main/java/org/apache/rat/Reporter.java @@ -28,7 +28,6 @@ import java.io.PrintWriter; import java.io.Writer; import java.nio.charset.StandardCharsets; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; @@ -45,6 +44,7 @@ import org.apache.rat.report.claim.ClaimStatistic; import org.apache.rat.report.xml.XmlReportFactory; import org.apache.rat.report.xml.writer.IXmlWriter; import org.apache.rat.report.xml.writer.XmlWriter; +import org.apache.rat.utils.StandardXmlFactory; import org.w3c.dom.Document; /** @@ -96,9 +96,9 @@ public class Reporter { report.endReport(); } InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); - document = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(inputStream); + document = StandardXmlFactory.documentBuilder().parse(inputStream); } else { - document = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument(); + document = StandardXmlFactory.documentBuilder().newDocument(); statistic = new ClaimStatistic(); } } catch (Exception e) { diff --git a/apache-rat-core/src/main/java/org/apache/rat/configuration/XMLConfigurationReader.java b/apache-rat-core/src/main/java/org/apache/rat/configuration/XMLConfigurationReader.java index 4d3e1a88..e238d3a8 100644 --- a/apache-rat-core/src/main/java/org/apache/rat/configuration/XMLConfigurationReader.java +++ b/apache-rat-core/src/main/java/org/apache/rat/configuration/XMLConfigurationReader.java @@ -37,8 +37,6 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.ImmutablePair; @@ -54,6 +52,7 @@ import org.apache.rat.configuration.builders.AbstractBuilder; import org.apache.rat.license.ILicense; import org.apache.rat.license.ILicenseFamily; import org.apache.rat.utils.DefaultLog; +import org.apache.rat.utils.StandardXmlFactory; import org.w3c.dom.DOMException; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -94,11 +93,7 @@ public final class XMLConfigurationReader implements LicenseReader, MatcherReade * Constructs the XML configuration reader. */ public XMLConfigurationReader() { - try { - document = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument(); - } catch (ParserConfigurationException e) { - throw new IllegalStateException("No XML parser defined", e); - } + document = StandardXmlFactory.documentBuilder().newDocument(); rootElement = document.createElement(XMLConfig.ROOT); document.appendChild(rootElement); familiesElement = document.createElement(XMLConfig.FAMILIES); @@ -151,15 +146,8 @@ public final class XMLConfigurationReader implements LicenseReader, MatcherReade * @param reader the reader to read XML from. */ public void read(final Reader reader) { - DocumentBuilder builder; try { - builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); - } catch (ParserConfigurationException e) { - throw new ConfigurationException("Unable to create DOM builder", e); - } - - try { - add(builder.parse(new InputSource(reader))); + add(StandardXmlFactory.documentBuilder().parse(new InputSource(reader))); } catch (SAXException | IOException e) { throw new ConfigurationException("Unable to read inputSource", e); } @@ -170,12 +158,7 @@ public final class XMLConfigurationReader implements LicenseReader, MatcherReade * @param uris The URIs to read. */ public void read(final URI... uris) { - DocumentBuilder builder; - try { - builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); - } catch (ParserConfigurationException e) { - throw new ConfigurationException("Unable to create DOM builder", e); - } + DocumentBuilder builder = StandardXmlFactory.documentBuilder(); for (URI uri : uris) { try (InputStream inputStream = uri.toURL().openStream()) { add(builder.parse(inputStream)); diff --git a/apache-rat-core/src/main/java/org/apache/rat/utils/StandardXmlFactory.java b/apache-rat-core/src/main/java/org/apache/rat/utils/StandardXmlFactory.java index 6ebf2397..7539213a 100644 --- a/apache-rat-core/src/main/java/org/apache/rat/utils/StandardXmlFactory.java +++ b/apache-rat-core/src/main/java/org/apache/rat/utils/StandardXmlFactory.java @@ -21,6 +21,9 @@ package org.apache.rat.utils; import java.io.InputStream; import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerConfigurationException; @@ -33,6 +36,13 @@ import javax.xml.transform.stream.StreamSource; */ public final class StandardXmlFactory { + /** + This is the PRIMARY defense. If DTDs (doctypes) are disallowed, almost all + XML entity attacks are prevented + */ + //Xerces 2 only - http://xerces.apache.org/xerces2-j/features.html#disallow-doctype-decl + private static final String FEATURE = "http://apache.org/xml/features/disallow-doctype-decl";; + private StandardXmlFactory() { // do not instantiate. } @@ -64,4 +74,24 @@ public final class StandardXmlFactory { transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount";, "4"); return transformer; } + + /** + * Creates a DocumentBuilder with reasonable security settings. + * @return a DocumentBuilder. + */ + public static DocumentBuilder documentBuilder() { + try { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setFeature(FEATURE, true); + + // and these as well, per Timothy Morgan's 2014 paper: "XML Schema, DTD, and Entity Attacks" + factory.setXIncludeAware(false); + + // remaining parser logic + + return factory.newDocumentBuilder(); + } catch (ParserConfigurationException e) { + throw new IllegalStateException("No XML parser defined", e); + } + } } diff --git a/apache-rat-core/src/test/java/org/apache/rat/ReporterOptionsProvider.java b/apache-rat-core/src/test/java/org/apache/rat/ReporterOptionsProvider.java index 2baa5e6f..09b35ffc 100644 --- a/apache-rat-core/src/test/java/org/apache/rat/ReporterOptionsProvider.java +++ b/apache-rat-core/src/test/java/org/apache/rat/ReporterOptionsProvider.java @@ -1036,7 +1036,7 @@ class ReporterOptionsProvider extends AbstractOptionsProvider implements Argumen throw new IllegalArgumentException("Unexpected filter: " + filter); } } - } catch (IOException | RatException | SAXException | ParserConfigurationException | + } catch (IOException | RatException | SAXException | XPathExpressionException e) { fail(e.getMessage(), e); } @@ -1085,7 +1085,7 @@ class ReporterOptionsProvider extends AbstractOptionsProvider implements Argumen throw new IllegalArgumentException("Unexpected filter: " + filter); } } - } catch (IOException | RatException | SAXException | ParserConfigurationException | + } catch (IOException | RatException | SAXException | XPathExpressionException e) { fail(e.getMessage(), e); } @@ -1143,7 +1143,7 @@ class ReporterOptionsProvider extends AbstractOptionsProvider implements Argumen throw new IllegalArgumentException("Unexpected processing " + proc); } } - } catch (IOException | RatException | SAXException | ParserConfigurationException | + } catch (IOException | RatException | SAXException | XPathExpressionException e) { fail(e.getMessage(), e); } @@ -1198,7 +1198,7 @@ class ReporterOptionsProvider extends AbstractOptionsProvider implements Argumen throw new IllegalArgumentException("Unexpected processing " + proc); } } - } catch (IOException | RatException | SAXException | ParserConfigurationException | + } catch (IOException | RatException | SAXException | XPathExpressionException e) { fail(e.getMessage(), e); } diff --git a/apache-rat-core/src/test/java/org/apache/rat/testhelpers/XmlUtils.java b/apache-rat-core/src/test/java/org/apache/rat/testhelpers/XmlUtils.java index bf6a7cbe..c34a027e 100644 --- a/apache-rat-core/src/test/java/org/apache/rat/testhelpers/XmlUtils.java +++ b/apache-rat-core/src/test/java/org/apache/rat/testhelpers/XmlUtils.java @@ -34,9 +34,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.FactoryConfigurationError; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParserFactory; import javax.xml.transform.OutputKeys; @@ -49,8 +46,10 @@ import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathExpressionException; +import org.apache.rat.api.RatException; import org.apache.rat.report.xml.writer.IXmlWriter; import org.apache.rat.utils.DefaultLog; +import org.apache.rat.utils.StandardXmlFactory; import org.w3c.dom.Document; import org.w3c.dom.NamedNodeMap; import org.w3c.dom.Node; @@ -133,12 +132,16 @@ public final class XmlUtils { return sb.toString(); } - public static Document toDom(final InputStream in) - throws SAXException, IOException, ParserConfigurationException, FactoryConfigurationError { - final DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); - Document result; - result = builder.parse(in); - return result; + /** + * Reads an input stream into a document. + * @param inputStream the input stream to read. + * @return the Document + * @throws SAXException on sax Error + * @throws IOException on IO Error + */ + public static Document toDom(final InputStream inputStream) + throws SAXException, IOException { + return StandardXmlFactory.documentBuilder().parse(inputStream); } public static void writeAttribute(final IXmlWriter writer, final String name, final boolean booleanValue) diff --git a/apache-rat-tasks/src/test/java/org/apache/rat/anttasks/ReportTest.java b/apache-rat-tasks/src/test/java/org/apache/rat/anttasks/ReportTest.java index 2f56caf1..ad44dda1 100644 --- a/apache-rat-tasks/src/test/java/org/apache/rat/anttasks/ReportTest.java +++ b/apache-rat-tasks/src/test/java/org/apache/rat/anttasks/ReportTest.java @@ -31,12 +31,12 @@ import java.util.Arrays; import java.util.Optional; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import org.apache.commons.io.IOUtils; import org.apache.rat.ReportConfiguration; import org.apache.rat.ReportConfigurationTest; import org.apache.rat.document.DocumentName; +import org.apache.rat.utils.StandardXmlFactory; import org.apache.tools.ant.BuildException; import org.apache.tools.ant.MagicNames; import org.apache.tools.ant.Target; @@ -269,7 +269,7 @@ public class ReportTest extends AbstractRatAntTaskTest { String outputDir = System.getProperty("output.dir", "target/anttasks"); String selftestOutput = System.getProperty("report.file", outputDir + "/selftest.report"); buildRule.executeTarget("testISO88591WithReportFile"); - DocumentBuilder db = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + DocumentBuilder db = StandardXmlFactory.documentBuilder(); boolean documentParsed; try (FileInputStream fis = new FileInputStream(selftestOutput)) { Document doc = db.parse(fis);
