This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/commons-xml.git
commit 115fe60626d6c0259c537a9a9b0774a55d1efc18 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Mon Apr 27 23:17:25 2026 +0200 Add support for Xalan (#8) * Tighten exception type checks in attack tests This change tightens `AttackTestSupport` to assert the specific JAXP exception type each helper expects, instead of catching `Exception` indiscriminately. Without this, the suite passed against an unrecognised JAXP implementation even when the thrown exception was not a parser-level rejection but an unrelated failure such as a `HardeningException` from the dispatch layer. * Adds support for Xalan Adds support for Apache Xalan as TrAX implementation. This implementation needs: 1. A deny-all `URIResolver` to prevent external resources from being loaded using XSLT-specific methods, 2. Wrappers to prevent SAX parsers from being configured from `SAXParserFactory.newInstance()` with merely FSP propagated on it. To harden the parsing of documents, we intercept `Source` arguments and replace them with arguments that are either `DOMSource` or have a preconfigured `XMLReader`. --- pom.xml | 118 +++++++++--- .../commons/xml/factory/DelegatingTemplates.java | 45 +++++ .../commons/xml/factory/DelegatingTransformer.java | 103 ++++++++++ .../xml/factory/DelegatingTransformerFactory.java | 133 +++++++++++++ .../apache/commons/xml/factory/XalanProvider.java | 208 +++++++++++++++++++++ .../apache/commons/xml/factory/XmlFactories.java | 5 + .../commons/xml/factory/AttackTestSupport.java | 127 ++++++++----- .../commons/xml/factory/ExternalDtdTest.java | 4 +- 8 files changed, 671 insertions(+), 72 deletions(-) diff --git a/pom.xml b/pom.xml index aa43ef6..e3245ca 100644 --- a/pom.xml +++ b/pom.xml @@ -54,9 +54,10 @@ <maven.compiler.target>1.8</maven.compiler.target> <!-- Optional JAXP implementations used by bundled providers. --> - <commons.xerces.version>2.12.2</commons.xerces.version> - <commons.woodstox.version>7.1.1</commons.woodstox.version> <commons.saxon.version>12.9</commons.saxon.version> + <commons.woodstox.version>7.1.1</commons.woodstox.version> + <commons.xalan.version>2.7.3</commons.xalan.version> + <commons.xerces.version>2.12.2</commons.xerces.version> </properties> <dependencies> @@ -78,12 +79,16 @@ <plugins> <!-- - Run the test suite four times, each with exactly one JAXP implementation on the classpath: + Run the test suite seven times, each with a distinct JAXP combination: - - test-stockjdk: stock JDK factories only - - test-xerces: stock JDK + external Apache Xerces - - test-woodstox: stock JDK + Woodstox (StAX) - - test-saxon: stock JDK + Saxon-HE (TrAX, XPath) + - test-saxon: Saxon-HE (TrAX, XPath) over stock JDK SAX + - test-saxon-xerces: Saxon-HE (TrAX, XPath) over stock JDK SAX or Apache Xerces SAX + (Saxon prefers the JDK SAX provider but may resolve to Xerces) + - test-stockjdk: stock JDK factories only (every group except xpath3) + - test-woodstox: Woodstox (StAX) + - test-xalan: Apache Xalan (TrAX, XPath) over stock JDK SAX + - test-xalan-xerces: Apache Xalan (TrAX, XPath) over Apache Xerces SAX + - test-xerces: Apache Xerces (DOM, SAX, Schema) --> <plugin> <groupId>org.apache.maven.plugins</groupId> @@ -109,24 +114,21 @@ <phase>none</phase> </execution> <execution> - <id>test-stockjdk</id> + <id>test-saxon</id> <goals><goal>test</goal></goals> <configuration> - <reportsDirectory>${project.build.directory}/surefire-reports/stockjdk</reportsDirectory> - <!-- Run every tagged and untagged test except xpath3, which needs Saxon on the classpath. --> - <excludedGroups>xpath3</excludedGroups> - <classpathDependencyExcludes> - <exclude>net.sf.saxon:Saxon-HE</exclude> - </classpathDependencyExcludes> + <reportsDirectory>${project.build.directory}/surefire-reports/saxon</reportsDirectory> + <!-- Saxon contributes TrAX (transformer + stylesheet) and XPath; xpath3 covers Saxon-only XPath 3.1 functions. --> + <groups>trax,xpath3</groups> </configuration> </execution> <execution> - <id>test-xerces</id> + <id>test-saxon-xerces</id> <goals><goal>test</goal></goals> <configuration> - <reportsDirectory>${project.build.directory}/surefire-reports/xerces</reportsDirectory> - <!-- Xerces only contributes DOM/SAX/Schema; the rest is JDK-bundled and already covered by test-stockjdk. --> - <groups>dom,sax,schema</groups> + <reportsDirectory>${project.build.directory}/surefire-reports/saxon-xerces</reportsDirectory> + <!-- Same groups as test-saxon, but with Apache Xerces also visible to Saxon's SAX-provider lookup. --> + <groups>trax,xpath3</groups> <additionalClasspathDependencies> <dependency> <groupId>xerces</groupId> @@ -134,6 +136,15 @@ <version>${commons.xerces.version}</version> </dependency> </additionalClasspathDependencies> + </configuration> + </execution> + <execution> + <id>test-stockjdk</id> + <goals><goal>test</goal></goals> + <configuration> + <reportsDirectory>${project.build.directory}/surefire-reports/stockjdk</reportsDirectory> + <!-- Run every tagged and untagged test except xpath3, which needs Saxon on the classpath. --> + <excludedGroups>xpath3</excludedGroups> <classpathDependencyExcludes> <exclude>net.sf.saxon:Saxon-HE</exclude> </classpathDependencyExcludes> @@ -159,12 +170,75 @@ </configuration> </execution> <execution> - <id>test-saxon</id> + <id>test-xalan</id> <goals><goal>test</goal></goals> <configuration> - <reportsDirectory>${project.build.directory}/surefire-reports/saxon</reportsDirectory> - <!-- Saxon contributes TrAX (transformer + stylesheet) and XPath; xpath3 covers Saxon-only XPath 3.1 functions. --> - <groups>trax,xpath3</groups> + <reportsDirectory>${project.build.directory}/surefire-reports/xalan</reportsDirectory> + <!-- Xalan contributes TrAX and XPath. --> + <groups>xpath,trax</groups> + <additionalClasspathDependencies> + <dependency> + <groupId>xalan</groupId> + <artifactId>serializer</artifactId> + <version>${commons.xalan.version}</version> + </dependency> + <dependency> + <groupId>xalan</groupId> + <artifactId>xalan</artifactId> + <version>${commons.xalan.version}</version> + </dependency> + </additionalClasspathDependencies> + <classpathDependencyExcludes> + <exclude>net.sf.saxon:Saxon-HE</exclude> + </classpathDependencyExcludes> + </configuration> + </execution> + <execution> + <id>test-xalan-xerces</id> + <goals><goal>test</goal></goals> + <configuration> + <reportsDirectory>${project.build.directory}/surefire-reports/xalan-xerces</reportsDirectory> + <!-- Same groups as test-xalan, but with Apache Xerces winning Xalan's internal SAX-provider lookup. --> + <groups>xpath,trax</groups> + <additionalClasspathDependencies> + <dependency> + <groupId>xalan</groupId> + <artifactId>serializer</artifactId> + <version>${commons.xalan.version}</version> + </dependency> + <dependency> + <groupId>xalan</groupId> + <artifactId>xalan</artifactId> + <version>${commons.xalan.version}</version> + </dependency> + <dependency> + <groupId>xerces</groupId> + <artifactId>xercesImpl</artifactId> + <version>${commons.xerces.version}</version> + </dependency> + </additionalClasspathDependencies> + <classpathDependencyExcludes> + <exclude>net.sf.saxon:Saxon-HE</exclude> + </classpathDependencyExcludes> + </configuration> + </execution> + <execution> + <id>test-xerces</id> + <goals><goal>test</goal></goals> + <configuration> + <reportsDirectory>${project.build.directory}/surefire-reports/xerces</reportsDirectory> + <!-- Xerces only contributes DOM/SAX/Schema; the rest is JDK-bundled and already covered by test-stockjdk. --> + <groups>dom,sax,schema</groups> + <additionalClasspathDependencies> + <dependency> + <groupId>xerces</groupId> + <artifactId>xercesImpl</artifactId> + <version>${commons.xerces.version}</version> + </dependency> + </additionalClasspathDependencies> + <classpathDependencyExcludes> + <exclude>net.sf.saxon:Saxon-HE</exclude> + </classpathDependencyExcludes> </configuration> </execution> </executions> diff --git a/src/main/java/org/apache/commons/xml/factory/DelegatingTemplates.java b/src/main/java/org/apache/commons/xml/factory/DelegatingTemplates.java new file mode 100644 index 0000000..11eb05d --- /dev/null +++ b/src/main/java/org/apache/commons/xml/factory/DelegatingTemplates.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.xml.factory; + +import java.util.Properties; + +import javax.xml.transform.Templates; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerConfigurationException; + +/** + * {@link Templates} wrapper that forwards every method to a wrapped delegate. + */ +class DelegatingTemplates implements Templates { + + private final Templates delegate; + + DelegatingTemplates(final Templates delegate) { + this.delegate = delegate; + } + + @Override + public Properties getOutputProperties() { + return delegate.getOutputProperties(); + } + + @Override + public Transformer newTransformer() throws TransformerConfigurationException { + return delegate.newTransformer(); + } +} diff --git a/src/main/java/org/apache/commons/xml/factory/DelegatingTransformer.java b/src/main/java/org/apache/commons/xml/factory/DelegatingTransformer.java new file mode 100644 index 0000000..2cd461d --- /dev/null +++ b/src/main/java/org/apache/commons/xml/factory/DelegatingTransformer.java @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.xml.factory; + +import java.util.Properties; + +import javax.xml.transform.ErrorListener; +import javax.xml.transform.Result; +import javax.xml.transform.Source; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerException; +import javax.xml.transform.URIResolver; + +/** + * {@link Transformer} subclass that forwards every method to a wrapped delegate. + */ +class DelegatingTransformer extends Transformer { + + private final Transformer delegate; + + DelegatingTransformer(final Transformer delegate) { + this.delegate = delegate; + } + + @Override + public void clearParameters() { + delegate.clearParameters(); + } + + @Override + public ErrorListener getErrorListener() { + return delegate.getErrorListener(); + } + + @Override + public Properties getOutputProperties() { + return delegate.getOutputProperties(); + } + + @Override + public String getOutputProperty(final String name) { + return delegate.getOutputProperty(name); + } + + @Override + public Object getParameter(final String name) { + return delegate.getParameter(name); + } + + @Override + public URIResolver getURIResolver() { + return delegate.getURIResolver(); + } + + @Override + public void reset() { + delegate.reset(); + } + + @Override + public void setErrorListener(final ErrorListener listener) { + delegate.setErrorListener(listener); + } + + @Override + public void setOutputProperties(final Properties properties) { + delegate.setOutputProperties(properties); + } + + @Override + public void setOutputProperty(final String name, final String value) { + delegate.setOutputProperty(name, value); + } + + @Override + public void setParameter(final String name, final Object value) { + delegate.setParameter(name, value); + } + + @Override + public void setURIResolver(final URIResolver resolver) { + delegate.setURIResolver(resolver); + } + + @Override + public void transform(final Source xmlSource, final Result outputTarget) throws TransformerException { + delegate.transform(xmlSource, outputTarget); + } +} diff --git a/src/main/java/org/apache/commons/xml/factory/DelegatingTransformerFactory.java b/src/main/java/org/apache/commons/xml/factory/DelegatingTransformerFactory.java new file mode 100644 index 0000000..edda54f --- /dev/null +++ b/src/main/java/org/apache/commons/xml/factory/DelegatingTransformerFactory.java @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.xml.factory; + +import javax.xml.transform.ErrorListener; +import javax.xml.transform.Source; +import javax.xml.transform.Templates; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerConfigurationException; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.URIResolver; +import javax.xml.transform.sax.SAXTransformerFactory; +import javax.xml.transform.sax.TemplatesHandler; +import javax.xml.transform.sax.TransformerHandler; + +import org.xml.sax.XMLFilter; + +/** + * {@link SAXTransformerFactory} subclass that forwards every method to a wrapped delegate. + */ +class DelegatingTransformerFactory extends SAXTransformerFactory { + + private final SAXTransformerFactory delegate; + + DelegatingTransformerFactory(final SAXTransformerFactory delegate) { + this.delegate = delegate; + } + + @Override + public Source getAssociatedStylesheet(final Source source, final String media, final String title, final String charset) + throws TransformerConfigurationException { + return delegate.getAssociatedStylesheet(source, media, title, charset); + } + + @Override + public Object getAttribute(final String name) { + return delegate.getAttribute(name); + } + + @Override + public ErrorListener getErrorListener() { + return delegate.getErrorListener(); + } + + @Override + public boolean getFeature(final String name) { + return delegate.getFeature(name); + } + + @Override + public URIResolver getURIResolver() { + return delegate.getURIResolver(); + } + + @Override + public Templates newTemplates(final Source source) throws TransformerConfigurationException { + return delegate.newTemplates(source); + } + + @Override + public TemplatesHandler newTemplatesHandler() throws TransformerConfigurationException { + return delegate.newTemplatesHandler(); + } + + @Override + public Transformer newTransformer() throws TransformerConfigurationException { + return delegate.newTransformer(); + } + + @Override + public Transformer newTransformer(final Source source) throws TransformerConfigurationException { + return delegate.newTransformer(source); + } + + @Override + public TransformerHandler newTransformerHandler() throws TransformerConfigurationException { + return delegate.newTransformerHandler(); + } + + @Override + public TransformerHandler newTransformerHandler(final Source source) throws TransformerConfigurationException { + return delegate.newTransformerHandler(source); + } + + @Override + public TransformerHandler newTransformerHandler(final Templates templates) throws TransformerConfigurationException { + return delegate.newTransformerHandler(templates); + } + + @Override + public XMLFilter newXMLFilter(final Source source) throws TransformerConfigurationException { + return delegate.newXMLFilter(source); + } + + @Override + public XMLFilter newXMLFilter(final Templates templates) throws TransformerConfigurationException { + return delegate.newXMLFilter(templates); + } + + @Override + public void setAttribute(final String name, final Object value) { + delegate.setAttribute(name, value); + } + + @Override + public void setErrorListener(final ErrorListener listener) { + delegate.setErrorListener(listener); + } + + @Override + public void setFeature(final String name, final boolean value) throws TransformerConfigurationException { + delegate.setFeature(name, value); + } + + @Override + public void setURIResolver(final URIResolver resolver) { + delegate.setURIResolver(resolver); + } +} diff --git a/src/main/java/org/apache/commons/xml/factory/XalanProvider.java b/src/main/java/org/apache/commons/xml/factory/XalanProvider.java new file mode 100644 index 0000000..d666960 --- /dev/null +++ b/src/main/java/org/apache/commons/xml/factory/XalanProvider.java @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.xml.factory; + +import static org.apache.commons.xml.factory.JaxpSetters.setFeature; + +import javax.xml.XMLConstants; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.Result; +import javax.xml.transform.Source; +import javax.xml.transform.Templates; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerConfigurationException; +import javax.xml.transform.TransformerException; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.dom.DOMSource; +import javax.xml.transform.sax.SAXSource; +import javax.xml.transform.sax.SAXTransformerFactory; +import javax.xml.transform.sax.TransformerHandler; +import javax.xml.xpath.XPathFactory; + +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.XMLReader; + +/** + * Hardening recipes for the external Apache Xalan distribution (the {@code xalan:xalan} artifact). + * + * <p>Factory classes live under the {@code org.apache.xalan.*} and {@code org.apache.xpath.*} packages. Xalan ships only TrAX and XPath; its DOM, SAX, StAX and + * Schema needs are served by whatever JDK or external Xerces is on the classpath.</p> + * + * <p>Hardening recipe applied to every factory below uses the same building blocks:</p> + * <ul> + * <li><strong>FSP</strong> ({@link XMLConstants#FEATURE_SECURE_PROCESSING}, set to {@code true}): enables Xalan's secure-processing mode, which disables + * reflection-based extension functions. Required.</li> + * <li><strong>{@link DenyAllResolver#URI}</strong>: required. Xalan does not implement the JAXP 1.5 {@code ACCESS_EXTERNAL_*} attributes; a deny-all + * {@link javax.xml.transform.URIResolver} blocks {@code xsl:include}, {@code xsl:import}, {@code xsl:source-document} during stylesheet compilation + * and {@code document()}, {@code unparsed-text()}, {@code collection()} at runtime.</li> + * <li> + * <p><strong>{@code Hardening*} wrappers + source rewriting</strong>: required. Xalan's source-document parsing path falls back to + * {@code SAXParserFactory.newInstance()} whenever the input is not a {@link DOMSource} or a {@link SAXSource} carrying its own {@link XMLReader}; it + * only sets FSP on the resulting reader. Three wrapper layers are needed:</p> + * <ol> + * <li>{@link HardeningTransformerFactory} rewrites the Source on every entry point that compiles a stylesheet.</li> + * <li>{@link HardeningTemplates} returns a hardened Transformer from {@link Templates#newTransformer()} so runtime source parsing is also + * covered.</li> + * <li>{@link HardeningTransformer} rewrites the Source on {@link Transformer#transform(Source, Result)}.</li> + * </ol> + * </li> + * </ul> + * + * <h2>Caveats</h2> + * <ul> + * <li>Replacing the {@code URIResolver} on the factory or transformer cancels the deny-all hardening for XSLT URI fetches; Xalan exposes no + * tamper-resistant equivalent of JAXP 1.5 {@code ACCESS_EXTERNAL_STYLESHEET}.</li> + * <li>The wrappers trust a {@link SAXSource} that carries its own {@link XMLReader}: the caller is expected to supply a hardened reader (via + * {@link XmlFactories#newSAXParserFactory()} or {@link XmlFactories#harden(XMLReader)}) in that case.</li> + * <li>{@link TransformerHandler} returned from {@code newTransformerHandler} is not wrapped: it processes incoming SAX events instead of reading a Source, + * so it has no inner Source-parsing path. A caller who pulls the inner {@link Transformer} via {@link TransformerHandler#getTransformer()} bypasses + * the runtime source rewrite.</li> + * </ul> + */ +final class XalanProvider { + + /** + * {@link TransformerFactory} wrapper that rewrites every Source-taking entry point through {@link #hardenSource(Source)} before delegating. + */ + private static final class HardeningTransformerFactory extends DelegatingTransformerFactory { + + HardeningTransformerFactory(final SAXTransformerFactory delegate) { + super(delegate); + } + + @Override + public Source getAssociatedStylesheet(final Source source, final String media, final String title, final String charset) + throws TransformerConfigurationException { + return super.getAssociatedStylesheet(hardenSource(source), media, title, charset); + } + + @Override + public Templates newTemplates(final Source source) throws TransformerConfigurationException { + final Templates templates = super.newTemplates(hardenSource(source)); + return templates == null ? null : new HardeningTemplates(templates); + } + + @Override + public Transformer newTransformer() throws TransformerConfigurationException { + // Identity transformer: still parses runtime sources, so wrap it to harden Transformer.transform(Source, Result). + final Transformer transformer = super.newTransformer(); + if (transformer == null) { + return null; + } + return new HardeningTransformer(transformer); + } + + @Override + public Transformer newTransformer(final Source source) throws TransformerConfigurationException { + final Transformer transformer = super.newTransformer(hardenSource(source)); + if (transformer == null) { + return null; + } + return new HardeningTransformer(transformer); + } + + @Override + public TransformerHandler newTransformerHandler(final Source source) throws TransformerConfigurationException { + return super.newTransformerHandler(hardenSource(source)); + } + } + + /** + * {@link Templates} wrapper whose only purpose is to return a {@link HardeningTransformer} from {@link Templates#newTransformer()}. + */ + private static final class HardeningTemplates extends DelegatingTemplates { + + HardeningTemplates(final Templates delegate) { + super(delegate); + } + + @Override + public Transformer newTransformer() throws TransformerConfigurationException { + final Transformer transformer = super.newTransformer(); + if (transformer == null) { + return null; + } + return new HardeningTransformer(transformer); + } + } + + /** + * {@link Transformer} wrapper that rewrites the Source on every {@link Transformer#transform(Source, Result)} call. + */ + private static final class HardeningTransformer extends DelegatingTransformer { + + HardeningTransformer(final Transformer delegate) { + super(delegate); + } + + @Override + public void transform(final Source xmlSource, final Result outputTarget) throws TransformerException { + super.transform(hardenSource(xmlSource), outputTarget); + } + } + + /** + * Rewrites a Source to one whose XML reader is hardened by {@link XmlFactories}. + * + * <p>Pass-through cases (the caller is trusted to have arranged equivalent hardening already):</p> + * <ul> + * <li>{@link DOMSource}: the tree is already built; no parser is invoked.</li> + * <li>{@link SAXSource} carrying its own {@link XMLReader}: the caller's reader is reused, matching Xalan's own contract.</li> + * </ul> + * + * <p>Otherwise the source is converted via {@link SAXSource#sourceToInputSource} and re-wrapped in a {@link SAXSource} backed by an + * {@link XmlFactories}-hardened reader.</p> + */ + private static Source hardenSource(final Source source) throws TransformerConfigurationException { + if (source instanceof DOMSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() != null) { + return source; + } + final InputSource inputSource = SAXSource.sourceToInputSource(source); + if (inputSource == null) { + return source; + } + try { + final XMLReader reader = XmlFactories.newSAXParserFactory().newSAXParser().getXMLReader(); + final SAXSource hardened = new SAXSource(reader, inputSource); + hardened.setSystemId(source.getSystemId()); + return hardened; + } catch (final ParserConfigurationException | SAXException e) { + throw new TransformerConfigurationException("Failed to obtain a hardened XMLReader for Xalan source parsing", e); + } + } + + static TransformerFactory configure(final TransformerFactory factory) { + // Required: enables Xalan's secure-processing mode (extension-function block) + setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); + // Required: Xalan does not honour JAXP 1.5 ACCESS_EXTERNAL_*; the deny-all URIResolver blocks: + // xsl:import/xsl:include at compile time and document()/unparsed-text() at runtime. + factory.setURIResolver(DenyAllResolver.URI); + // Required: Xalan's internal SAX reader is sourced from SAXParserFactory.newInstance() and only carries FSP. + // We replace it with our hardened factory + return new HardeningTransformerFactory((SAXTransformerFactory) factory); + } + + static XPathFactory configure(final XPathFactory factory) { + // Required: enables Xalan's secure-processing mode; XPathFactory has no property API for finer control. + setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); + return factory; + } + + private XalanProvider() { + } +} diff --git a/src/main/java/org/apache/commons/xml/factory/XmlFactories.java b/src/main/java/org/apache/commons/xml/factory/XmlFactories.java index 0519a27..3ce0e0b 100644 --- a/src/main/java/org/apache/commons/xml/factory/XmlFactories.java +++ b/src/main/java/org/apache/commons/xml/factory/XmlFactories.java @@ -102,6 +102,9 @@ private static TransformerFactory dispatch(final TransformerFactory factory) { switch (factory.getClass().getName()) { case "com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl": return StockJdkProvider.configure(factory); + case "org.apache.xalan.processor.TransformerFactoryImpl": + case "org.apache.xalan.xsltc.trax.TransformerFactoryImpl": + return XalanProvider.configure(factory); case "net.sf.saxon.TransformerFactoryImpl": case "com.saxonica.config.ProfessionalTransformerFactory": case "com.saxonica.config.EnterpriseTransformerFactory": @@ -115,6 +118,8 @@ private static XPathFactory dispatch(final XPathFactory factory) { switch (factory.getClass().getName()) { case "com.sun.org.apache.xpath.internal.jaxp.XPathFactoryImpl": return StockJdkProvider.configure(factory); + case "org.apache.xpath.jaxp.XPathFactoryImpl": + return XalanProvider.configure(factory); case "net.sf.saxon.xpath.XPathFactoryImpl": return SaxonProvider.configure(factory); default: diff --git a/src/test/java/org/apache/commons/xml/factory/AttackTestSupport.java b/src/test/java/org/apache/commons/xml/factory/AttackTestSupport.java index dc7c914..a35ad0a 100644 --- a/src/test/java/org/apache/commons/xml/factory/AttackTestSupport.java +++ b/src/test/java/org/apache/commons/xml/factory/AttackTestSupport.java @@ -25,6 +25,7 @@ import java.io.StringReader; import java.io.StringWriter; import java.net.URL; +import java.util.Arrays; import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; @@ -33,10 +34,12 @@ import javax.xml.parsers.SAXParserFactory; import javax.xml.stream.XMLEventReader; import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import javax.xml.transform.Source; import javax.xml.transform.Templates; import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; import javax.xml.transform.sax.SAXSource; import javax.xml.transform.stream.StreamResult; @@ -44,8 +47,11 @@ import javax.xml.validation.SchemaFactory; import javax.xml.validation.Validator; +import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.api.function.ThrowingSupplier; import org.w3c.dom.Document; import org.xml.sax.InputSource; +import org.xml.sax.SAXException; import org.xml.sax.XMLReader; import org.xml.sax.helpers.DefaultHandler; @@ -71,27 +77,11 @@ * resource form preserves the system id so relative {@code xs:include} / {@code xs:import} / {@code xs:redefine} / {@code xsl:include} / {@code xsl:import} * URIs resolve normally.</p> * - * <p>The two generic primitives {@link #assertParseFails(ThrowingAction, String)} and {@link #assertParseSucceeds(ThrowingAction, String)} are exposed for - * tests that need to compose a non-standard factory call.</p> + * <p>The two generic primitives {@link #assertParseFails} and {@link #assertParseSucceeds} are exposed for tests that need to compose a non-standard factory + * call.</p> */ final class AttackTestSupport { - /** - * Action that may throw any exception; lets test bodies forward checked JAXP exceptions. - */ - @FunctionalInterface - interface ThrowingAction { - void run() throws Exception; - } - - /** - * Action that may throw any exception and returns the captured output text used for the leaked-marker check. - */ - @FunctionalInterface - interface ThrowingStringSupplier { - String get() throws Exception; - } - /** * Trivial W3C XML Schema that validates {@link #xmlBody(String)} output. */ @@ -126,7 +116,7 @@ interface ThrowingStringSupplier { * <p>{@link DocumentBuilder#parse(InputSource)} via {@link XmlFactories#newDocumentBuilderFactory()}; only a thrown exception passes.</p> */ static void assertDomBlocks(final String payload) { - assertParseFails(() -> XmlFactories.newDocumentBuilderFactory().newDocumentBuilder().parse(inputSource(payload)), "DOM"); + assertParseFails(() -> XmlFactories.newDocumentBuilderFactory().newDocumentBuilder().parse(inputSource(payload)), "DOM", SAXException.class); } /** @@ -139,7 +129,7 @@ static void assertDomDoesNotLeak(final String payload) { assertNoLeak(() -> { final Document doc = XmlFactories.newDocumentBuilderFactory().newDocumentBuilder().parse(inputSource(payload)); return doc.getDocumentElement() == null ? "" : doc.getDocumentElement().getTextContent(); - }, "DOM"); + }, "DOM", SAXException.class); } /** @@ -169,29 +159,53 @@ static void assertDomResolves(final String payload) { /** * Skeleton for every {@code assert*DoesNotLeak} helper. * - * <p>Treats any thrown exception as "hardening blocked at parse" (acceptable); otherwise asserts the captured output omits {@link #LEAKED_MARKER}.</p> + * <p>Treats a thrown exception of one of the {@code expected} types as "hardening blocked at parse" (acceptable); otherwise asserts the captured output + * omits {@link #LEAKED_MARKER}. A throw whose type does not match {@code expected} fails the test, so unrelated failures (e.g. a {@link HardeningException} + * because no recipe matched the JAXP implementation) cannot be silently accepted as a clean block.</p> + * + * @param action the parse to execute, returning the captured output text checked for {@link #LEAKED_MARKER}. + * @param description short label naming the JAXP surface under test. + * @param expected the exception types any of which the hardening layer may surface as a clean rejection. */ - private static void assertNoLeak(final ThrowingStringSupplier action, final String description) { + @SafeVarargs + private static void assertNoLeak(final ThrowingSupplier<String> action, final String description, final Class<? extends Throwable>... expected) { final String output; try { output = action.get(); - } catch (final Exception e) { - return; // hardening blocked at parse; acceptable outcome. + } catch (final Throwable thrown) { + if (Arrays.stream(expected).anyMatch(c -> c.isInstance(thrown))) { + return; // hardening blocked at parse; acceptable outcome. + } + throw new AssertionError(blockedDescription(description) + " (got " + thrown.getClass().getName() + ")", thrown); } assertFalse(output.contains(LEAKED_MARKER), "Hardening did not block " + description + "; output contained marker '" + LEAKED_MARKER + "'.\nFull output:\n" + output); } /** - * Asserts the supplied action throws. - * - * <p>Generic primitive underlying every {@code assert*Blocks(...)} helper; exposed for tests that compose a non-standard hardened-side call.</p> + * Asserts the supplied parsing action throws an exception of the {@code expected} type. * * @param action the parse to execute. - * @param description short label included in the failure message. + * @param description short label naming the JAXP surface under test. + * @param expected the exception type the hardening layer is expected to surface. */ - static void assertParseFails(final ThrowingAction action, final String description) { - assertThrows(Exception.class, action::run, "Hardening did not block " + description + "; parse completed successfully."); + static void assertParseFails(final Executable action, final String description, final Class<? extends Throwable> expected) { + assertThrows(expected, action, blockedDescription(description)); + } + + /** + * Asserts the supplied parsing action throws an exception that matches one of the {@code expected} types. + * + * @param action the parse to execute. + * @param description short label naming the JAXP surface under test. + * @param expected the exception types any of which the hardening layer may surface. + */ + @SafeVarargs + static void assertParseFails(final Executable action, final String description, final Class<? extends Throwable>... expected) { + final Throwable thrown = assertThrows(Exception.class, action, blockedDescription(description)); + if (Arrays.stream(expected).noneMatch(c -> c.isInstance(thrown))) { + throw new AssertionError(blockedDescription(description) + " (got " + thrown.getClass().getName() + ")", thrown); + } } /** @@ -203,8 +217,8 @@ static void assertParseFails(final ThrowingAction action, final String descripti * @param action the parse to execute. * @param description short label included in the failure message. */ - static void assertParseSucceeds(final ThrowingAction action, final String description) { - assertDoesNotThrow(action::run, "Unconfigured factory should parse " + description + "; the wrapper or its external reference is broken."); + static void assertParseSucceeds(final Executable action, final String description) { + assertDoesNotThrow(action, "Unconfigured factory should parse " + description + "; the wrapper or its external reference is broken."); } /** @@ -213,7 +227,7 @@ static void assertParseSucceeds(final ThrowingAction action, final String descri * <p>{@link XMLReader#parse(InputSource)} on a parser from {@link XmlFactories#newSAXParserFactory()}; only a thrown exception passes.</p> */ static void assertSaxBlocks(final String payload) { - assertParseFails(() -> parseQuietly(XmlFactories.newSAXParserFactory().newSAXParser().getXMLReader(), payload), "SAX"); + assertParseFails(() -> parseQuietly(XmlFactories.newSAXParserFactory().newSAXParser().getXMLReader(), payload), "SAX", SAXException.class); } /** @@ -223,7 +237,7 @@ static void assertSaxBlocks(final String payload) { * entities when an external subset is declared but unread.</p> */ static void assertSaxDoesNotLeak(final String payload) { - assertNoLeak(() -> captureCharacters(XmlFactories.newSAXParserFactory().newSAXParser().getXMLReader(), payload), "SAX"); + assertNoLeak(() -> captureCharacters(XmlFactories.newSAXParserFactory().newSAXParser().getXMLReader(), payload), "SAX", SAXException.class); } /** @@ -257,7 +271,7 @@ static void assertSaxResolves(final String payload) { * <p>{@link SchemaFactory#newSchema(Source)} via {@link XmlFactories#newSchemaFactory()}; only a thrown exception passes.</p> */ static void assertSchemaBlocks(final Source xsd) { - assertParseFails(() -> XmlFactories.newSchemaFactory().newSchema(xsd), "Schema compile"); + assertParseFails(() -> XmlFactories.newSchemaFactory().newSchema(xsd), "Schema compile", SAXException.class, SecurityException.class); } /** @@ -282,8 +296,8 @@ static void assertSchemaCompiles(final Source xsd) { * throw.</p> */ static void assertStaxBlocks(final String payload) { - assertParseFails(() -> consumeStreamReader(XmlFactories.newXMLInputFactory(), payload), "StAX stream"); - assertParseFails(() -> consumeEventReader(XmlFactories.newXMLInputFactory(), payload), "StAX event"); + assertParseFails(() -> consumeStreamReader(XmlFactories.newXMLInputFactory(), payload), "StAX stream", XMLStreamException.class); + assertParseFails(() -> consumeEventReader(XmlFactories.newXMLInputFactory(), payload), "StAX event", XMLStreamException.class); } /** @@ -319,8 +333,12 @@ static void assertStaxResolves(final String payload) { static void assertTemplatesBlocks(final Source xslt) { assertParseFails(() -> { final Templates templates = XmlFactories.newTransformerFactory().newTemplates(xslt); + // Xalan return `null` if the template fails + if (templates == null) { + throw new TransformerException("Transformer factory returned null"); + } templates.newTransformer().transform(streamSource("<root/>"), new StreamResult(new StringWriter())); - }, "Templates"); + }, "Templates", TransformerException.class); } /** @@ -347,9 +365,12 @@ static void assertTemplatesDoesNotLeak(final Source xslt) { assertNoLeak(() -> { final StringWriter sink = new StringWriter(); final Templates templates = XmlFactories.newTransformerFactory().newTemplates(xslt); - templates.newTransformer().transform(streamSource("<root/>"), new StreamResult(sink)); + // Xalan return `null` if the template fails + if (templates != null) { + templates.newTransformer().transform(streamSource("<root/>"), new StreamResult(sink)); + } return sink.toString(); - }, "Templates"); + }, "Templates", TransformerException.class); } /** @@ -361,7 +382,7 @@ static void assertTemplatesDoesNotLeak(final Source xslt) { static void assertTransformerBlocks(final String payload) { assertParseFails( () -> XmlFactories.newTransformerFactory().newTransformer().transform(streamSource(payload), new StreamResult(new StringWriter())), - "Transformer"); + "Transformer", TransformerException.class); } /** @@ -375,7 +396,7 @@ static void assertTransformerDoesNotLeak(final String payload) { final StringWriter sink = new StringWriter(); XmlFactories.newTransformerFactory().newTransformer().transform(streamSource(payload), new StreamResult(sink)); return sink.toString(); - }, "Transformer"); + }, "Transformer", TransformerException.class); } /** @@ -416,7 +437,7 @@ static void assertValidatorAccepts(final String xml) { static void assertValidatorBlocks(final String xml) { assertParseFails( () -> XmlFactories.newSchemaFactory().newSchema(streamSource(BENIGN_SCHEMA)).newValidator().validate(streamSource(xml)), - "Validator"); + "Validator", SAXException.class, SecurityException.class); } /** @@ -425,7 +446,7 @@ static void assertValidatorBlocks(final String xml) { * <p>{@link XMLReader#parse(InputSource)} on a raw reader hardened via {@link XmlFactories#harden(XMLReader)}; only a thrown exception passes.</p> */ static void assertXmlReaderBlocks(final String payload) { - assertParseFails(() -> parseQuietly(rawHardenedReader(), payload), "XMLReader"); + assertParseFails(() -> parseQuietly(rawHardenedReader(), payload), "XMLReader", SAXException.class); } /** @@ -435,7 +456,7 @@ static void assertXmlReaderBlocks(final String payload) { * undeclared entities when an external subset is declared but unread.</p> */ static void assertXmlReaderDoesNotLeak(final String payload) { - assertNoLeak(() -> captureCharacters(rawHardenedReader(), payload), "XMLReader"); + assertNoLeak(() -> captureCharacters(rawHardenedReader(), payload), "XMLReader", SAXException.class); } /** @@ -448,6 +469,16 @@ static void assertXmlReaderParses(final String payload) { assertParseSucceeds(() -> parseQuietly(rawHardenedReader(), payload), "XMLReader"); } + /** + * Builds the failure message used by every {@code assert*Blocks(...)} helper. + * + * @param description short label naming the JAXP surface under test. + * @return the assertion-failure message string. + */ + private static String blockedDescription(final String description) { + return "Hardening did not block " + description + "; parse completed successfully."; + } + /** Parses the payload through the supplied reader and returns the accumulated character data, used by the SAX-based {@code DoesNotLeak} helpers. */ private static String captureCharacters(final XMLReader reader, final String payload) throws Exception { final StringBuilder text = new StringBuilder(); @@ -537,10 +568,10 @@ static StreamSource streamSource(final String xml) { } /** Runs the action and silently swallows any thrown exception; used to apply best-effort permissive-side flags that may not be supported. */ - private static void suppressException(final ThrowingAction action) { + private static void suppressException(final Executable action) { try { - action.run(); - } catch (final Exception e) { + action.execute(); + } catch (final Throwable e) { // Ignore } } diff --git a/src/test/java/org/apache/commons/xml/factory/ExternalDtdTest.java b/src/test/java/org/apache/commons/xml/factory/ExternalDtdTest.java index bce4f30..8662032 100644 --- a/src/test/java/org/apache/commons/xml/factory/ExternalDtdTest.java +++ b/src/test/java/org/apache/commons/xml/factory/ExternalDtdTest.java @@ -90,8 +90,8 @@ void hardenedTemplatesBlocks() { @Test @Tag("trax") - void hardenedTransformerBlocks() { - AttackTestSupport.assertTransformerBlocks(xmlPayload()); + void hardenedTransformerDoesNotLeak() { + AttackTestSupport.assertTransformerDoesNotLeak(xmlPayload()); } @Test
