This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch feature/ignore-all-resolver-floors in repository https://gitbox.apache.org/repos/asf/commons-xml.git
commit 26c1acf3c6e763f9b7a46356f25f24b296bd3a0b Author: Piotr P. Karwasz <[email protected]> AuthorDate: Sat Jul 11 14:14:56 2026 +0200 Make the resolver floors ignore-all instead of deny-all An external reference the caller's resolver does not resolve is now resolved to empty content rather than rejected with an exception. Nothing is fetched either way, so the security property is unchanged; the difference is that a parse now continues with empty content instead of failing. Rename the four floors accordingly and collapse the variants the deny/ignore split required: FallbackDenyEntityResolver2 -> FallbackIgnoreEntityResolver2 FallbackDenyXMLResolver -> FallbackIgnoreXMLResolver FallbackDenyLSResourceResolver -> FallbackIgnoreLSResourceResolver FallbackDenyURIResolver -> FallbackIgnoreURIResolver `SAXParserHardener.DtdAwareDenyResolver` and `StaxHardener.DtdSubsetFloor` existed only to exempt the external DTD subset from the deny, and the old `FallbackIgnoreXMLResolver` only to exempt Woodstox's undeclared entities; with ignore as the default all three are redundant. Removing them, together with the now-dead `HardeningException.forbidden`, takes the shade closure from 33 classes to 30. StAX collapses further: the floor moves into the `HardeningXMLInputFactory` constructor (as `HardeningXMLReader` already did), a single `setXMLResolver` covers both the JDK Zephyr and Woodstox (whose `setXMLResolver` fans out to its DTD-subset and entity resolvers), and the Zephyr `ignore-external-dtd` property is dropped because the floor already empties the subset. Only Woodstox's undeclared-entity hook stays separate: emptying the external subset leaves the entities it declared undeclared, and that hook is outside the fan-out. Tests that asserted an exception now assert the external resource does not leak. Where the outcome differs by implementation (Saxon still rejects through ALLOWED_PROTOCOLS, an emptied schema import fails to compile) they accept either outcome through the new assert*BlocksOrDoesNotLeak helpers. Assisted-By: Claude Opus 4.8 (1M context) <[email protected]> --- .../commons/xml/DocumentBuilderHardener.java | 6 +- .../commons/xml/FallbackDenyXMLResolver.java | 84 --------------------- ...er2.java => FallbackIgnoreEntityResolver2.java} | 37 ++++----- ....java => FallbackIgnoreLSResourceResolver.java} | 27 +++++-- ...esolver.java => FallbackIgnoreURIResolver.java} | 18 ++--- .../commons/xml/FallbackIgnoreXMLResolver.java | 28 +++++-- .../commons/xml/HardeningDocumentBuilder.java | 8 +- .../xml/HardeningDocumentBuilderFactory.java | 2 +- .../org/apache/commons/xml/HardeningException.java | 15 ---- .../org/apache/commons/xml/HardeningSchema.java | 2 +- .../apache/commons/xml/HardeningSchemaFactory.java | 6 +- .../apache/commons/xml/HardeningTransformer.java | 8 +- .../commons/xml/HardeningTransformerFactory.java | 2 +- .../org/apache/commons/xml/HardeningValidator.java | 8 +- .../commons/xml/HardeningValidatorHandler.java | 6 +- .../commons/xml/HardeningXMLInputFactory.java | 30 +++++--- .../org/apache/commons/xml/HardeningXMLReader.java | 16 ++-- .../org/apache/commons/xml/SAXParserHardener.java | 87 +++------------------- .../org/apache/commons/xml/SchemaHardener.java | 2 +- .../java/org/apache/commons/xml/StaxHardener.java | 67 +---------------- .../apache/commons/xml/TransformerHardener.java | 4 +- .../org/apache/commons/xml/AttackTestSupport.java | 71 +++++++++++++++++- .../commons/xml/EntityResolverFloorTest.java | 67 ++++++++++++----- .../commons/xml/ExternalGeneralEntityTest.java | 16 ++-- .../commons/xml/ExternalParameterEntityTest.java | 16 ++-- .../apache/commons/xml/SchemaLocationDomTest.java | 28 +++---- .../commons/xml/SchemaLocationPropertyTest.java | 2 +- .../apache/commons/xml/SchemaLocationSaxTest.java | 31 ++++---- .../apache/commons/xml/ShadingFootprintTest.java | 21 +++--- .../commons/xml/TransformerDocumentTest.java | 2 +- .../java/org/apache/commons/xml/XIncludeTest.java | 48 ++++++++---- 31 files changed, 346 insertions(+), 419 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/DocumentBuilderHardener.java b/src/main/java/org/apache/commons/xml/DocumentBuilderHardener.java index a97af17..6cf9c6a 100644 --- a/src/main/java/org/apache/commons/xml/DocumentBuilderHardener.java +++ b/src/main/java/org/apache/commons/xml/DocumentBuilderHardener.java @@ -34,8 +34,8 @@ * <li><strong>FSP</strong>: required. It switches on the implementation's built-in security manager, which is what carries the processing limits.</li> * <li><strong>{@code XERCES_LOAD_EXTERNAL_DTD}</strong>: optional. Where supported, it skips the external DTD subset on non-validating parsers so a * DOCTYPE-only document parses without a fetch attempt. If not supported, the fetch will throw instead, due to the following settings.</li> - * <li><strong>Deny-all resolver floor</strong>: every produced {@link DocumentBuilder} is wrapped by a {@link HardeningDocumentBuilderFactory} that keeps a - * deny-all {@link EntityResolver} floor. That floor blocks external DTD, entity, schema and {@code xi:include} fetches in one place: the stock JDK's + * <li><strong>Ignore-all resolver floor</strong>: every produced {@link DocumentBuilder} is wrapped by a {@link HardeningDocumentBuilderFactory} that keeps an + * ignore-all {@link EntityResolver} floor. That floor blocks external DTD, entity, schema and {@code xi:include} fetches in one place: the stock JDK's * XInclude processor ignores {@code ACCESS_EXTERNAL_*} and consults the {@link EntityResolver} instead, so no {@code ACCESS_EXTERNAL_*} attributes are * needed here. A caller can chain its own resolver onto the floor to allow-list resources, but cannot remove it.</li> * </ul> @@ -57,7 +57,7 @@ static DocumentBuilderFactory harden(final DocumentBuilderFactory factory) { setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Optional: skip the external DTD subset on non-validating parsers so DOCTYPE-only documents parse without a blocked fetch attempt. setOptionalFeature(factory, XERCES_LOAD_EXTERNAL_DTD, false); - // Required: HardeningDocumentBuilderFactory installs a deny-all EntityResolver floor on every DocumentBuilder. + // Required: HardeningDocumentBuilderFactory installs an ignore-all EntityResolver floor on every DocumentBuilder. // That floor blocks external DTD, entity, schema and xi:include fetches in one place: no ACCESS_EXTERNAL_* attributes are needed here. // Callers can chain their resolvers, but not override the floor. return new HardeningDocumentBuilderFactory(factory); diff --git a/src/main/java/org/apache/commons/xml/FallbackDenyXMLResolver.java b/src/main/java/org/apache/commons/xml/FallbackDenyXMLResolver.java deleted file mode 100644 index b5e1812..0000000 --- a/src/main/java/org/apache/commons/xml/FallbackDenyXMLResolver.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * 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; - -import javax.xml.stream.XMLResolver; -import javax.xml.stream.XMLStreamException; -import javax.xml.transform.Source; - -/** - * {@link XMLResolver} floor: consults an optional caller-supplied resolver and denies (throws) whatever the caller does not resolve. - * - * <p>The StAX counterpart of {@link FallbackDenyEntityResolver2}, installed on each entity-resolution hook. The hardened {@link javax.xml.stream.XMLInputFactory} - * wrapper routes a caller-set resolver through {@link #setDelegate} rather than letting it replace the floor. A caller opts a specific entity in by returning - * a non-{@code null} result; anything left unresolved goes to {@link #onUnresolved}, which denies by default. Subclasses override {@code onUnresolved} to give - * a hook a different unresolved policy (e.g. return an empty input for the external DTD subset, or for undeclared entities) while keeping the caller-delegate - * behavior.</p> - */ -class FallbackDenyXMLResolver implements XMLResolver { - - private XMLResolver delegate; - - FallbackDenyXMLResolver(final XMLResolver delegate) { - this.delegate = delegate; - } - - final void setDelegate(final XMLResolver delegate) { - this.delegate = delegate; - } - - final XMLResolver getDelegate() { - return delegate; - } - - @Override - public final Object resolveEntity(final String publicID, final String systemID, final String baseURI, final String namespace) throws XMLStreamException { - final Object resolved = delegate != null ? delegate.resolveEntity(publicID, systemID, baseURI, namespace) : null; - return resolved != null ? resolved : onUnresolved(publicID, systemID, baseURI, namespace); - } - - /** - * Outcome when the caller delegate does not resolve the entity. Denies by default; a subclass may return an {@link java.io.InputStream}, {@link Source} or - * other {@link XMLResolver}-supported value (for example an empty input) instead of calling {@code super}, or {@code throw} - * {@link #denied(String, String, String, String)} to deny only some lookups. - * - * @param publicID The public identifier, or {@code null} if none. - * @param systemID The system identifier of the unresolved entity. - * @param baseURI The base URI for relative resolution, or {@code null}. - * @param namespace The namespace (or, for Woodstox, the entity name), or {@code null}. - * @return The replacement input, or a value the caller's parser accepts; the default implementation never returns normally. - * @throws XMLStreamException to deny the lookup (the default behavior). - */ - protected Object onUnresolved(final String publicID, final String systemID, final String baseURI, final String namespace) throws XMLStreamException { - throw denied(publicID, systemID, baseURI, namespace); - } - - /** - * Builds the standard "forbidden by hardening" exception for a denied lookup, so a subclass with a mixed policy can reuse the deny outcome for the - * lookups it refuses. - * - * @param publicID The public identifier, or {@code null} if none. - * @param systemID The system identifier of the unresolved entity. - * @param baseURI The base URI for relative resolution, or {@code null}. - * @param namespace The namespace (or, for Woodstox, the entity name), or {@code null}. - * @return The exception to throw. - */ - protected final XMLStreamException denied(final String publicID, final String systemID, final String baseURI, final String namespace) { - return new XMLStreamException(HardeningException.forbidden(null, namespace, publicID, systemID, baseURI)); - } -} diff --git a/src/main/java/org/apache/commons/xml/FallbackDenyEntityResolver2.java b/src/main/java/org/apache/commons/xml/FallbackIgnoreEntityResolver2.java similarity index 78% rename from src/main/java/org/apache/commons/xml/FallbackDenyEntityResolver2.java rename to src/main/java/org/apache/commons/xml/FallbackIgnoreEntityResolver2.java index 92ccf4f..d2aa6e1 100644 --- a/src/main/java/org/apache/commons/xml/FallbackDenyEntityResolver2.java +++ b/src/main/java/org/apache/commons/xml/FallbackIgnoreEntityResolver2.java @@ -17,6 +17,7 @@ package org.apache.commons.xml; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -28,31 +29,31 @@ import org.xml.sax.ext.EntityResolver2; /** - * Entity resolver that consults an optional caller-supplied resolver and denies (throws) whatever the caller does not resolve. + * Entity resolver that consults an optional caller-supplied resolver and ignores (resolves to empty) whatever the caller does not resolve. * * <p>The canonical hardening floor, and the entity-resolution counterpart of the JAXP 1.5 {@code ACCESS_EXTERNAL_*} properties. Every floor - * ({@link FallbackDenyLSResourceResolver}, {@link FallbackDenyURIResolver}, {@link FallbackDenyXMLResolver} and its {@link FallbackIgnoreXMLResolver} variant) - * shares two defining properties:</p> + * ({@link FallbackIgnoreLSResourceResolver}, {@link FallbackIgnoreURIResolver} and {@link FallbackIgnoreXMLResolver}) shares two defining properties:</p> * <ol> * <li><strong>Non-removable, and it wraps the resolver the caller sets.</strong> The hardened wrappers install one and route a caller-set resolver through * {@code setDelegate} rather than letting it replace the floor, so the caller's resolver is consulted first but cannot remove the floor underneath it.</li> * <li><strong>It supplies the default action for a lookup the caller's resolver does not resolve</strong> (a {@code null} return, or no caller resolver at * all). This is where a floor departs from stock JAXP: normally an unresolved lookup falls back to the processor's built-in resolution and the resource is - * <em>fetched</em>; a floor instead <em>denies</em> it (throws).</li> + * <em>fetched</em>; a floor instead resolves it to <em>empty</em> content, so the parse continues without the external fetch and without a leak.</li> * </ol> * * <p>The hardened DOM and SAX wrappers install one of these and, when the caller sets their own {@link EntityResolver}, route it through {@link #setDelegate} * rather than letting it replace the floor. A caller therefore opts a specific resource in by returning a non-{@code null} {@link InputSource} from their - * resolver; anything they leave unresolved (a {@code null} return, or no caller resolver at all) goes to {@link #onUnresolved}, which denies by default.</p> + * resolver; anything they leave unresolved (a {@code null} return, or no caller resolver at all) goes to {@link #onUnresolved}, which returns empty content by + * default.</p> * - * <p>It extends {@link DefaultHandler2} so it is also usable as a {@link org.xml.sax.ext.LexicalHandler} (see {@code SAXParserHardener}'s Android subclass, - * which needs {@code startDTD}/{@code endDTD}); {@link #getExternalSubset} therefore inherits the {@code DefaultHandler2} "no synthetic subset" default. Only - * {@link #resolveEntity(String, String, String, String) resolveEntity} (the actual external fetch) reaches the deny fallback.</p> + * <p>It extends {@link DefaultHandler2} so it is also usable as a {@link org.xml.sax.ext.LexicalHandler}; {@link #getExternalSubset} therefore inherits the + * {@code DefaultHandler2} "no synthetic subset" default. Only {@link #resolveEntity(String, String, String, String) resolveEntity} (the actual external fetch) + * reaches the ignore fallback.</p> */ -class FallbackDenyEntityResolver2 extends DefaultHandler2 { +class FallbackIgnoreEntityResolver2 extends DefaultHandler2 { /** - * Caller-supplied resolver consulted first, or {@code null} for a pure deny-all floor. + * Caller-supplied resolver consulted first, or {@code null} for a pure ignore-all floor. */ private EntityResolver delegate; @@ -75,14 +76,14 @@ private static String absolutize(final String baseURI, final String systemId) { } } - FallbackDenyEntityResolver2(final EntityResolver delegate) { + FallbackIgnoreEntityResolver2(final EntityResolver delegate) { this.delegate = delegate; } /** * Replaces the caller resolver consulted ahead of the floor; lets a single floor instance back successive {@code setEntityResolver} calls. * - * @param delegate The caller-supplied resolver, or {@code null} for a pure deny-all floor. + * @param delegate The caller-supplied resolver, or {@code null} for a pure ignore-all floor. */ final void setDelegate(final EntityResolver delegate) { this.delegate = delegate; @@ -105,20 +106,20 @@ public final InputSource resolveEntity(final String name, final String publicId, } /** - * Outcome when neither the caller delegate nor this resolver provides the entity. Denies by default; a subclass may permit specific lookups (e.g. the - * external DTD subset) by returning {@code null} or an {@link InputSource} instead of calling {@code super}. + * Outcome when neither the caller delegate nor this resolver provides the entity. Resolves to empty content by default, so the external resource is neither + * fetched nor leaked and the parse continues with no replacement text. * * @param name The entity name, or {@code null} on the 2-arg resolution path. * @param publicId The public identifier, or {@code null} if none. * @param baseURI The base URI for relative resolution, or {@code null}. * @param systemId The system identifier of the unresolved entity. - * @return An {@link InputSource} to permit the lookup, or {@code null} to skip it silently; the default implementation never returns normally. - * @throws SAXException to deny the lookup (the default behavior). - * @throws IOException if a subclass opens a stream that fails. + * @return An empty {@link InputSource}. + * @throws SAXException never by the default implementation. + * @throws IOException never by the default implementation. */ protected InputSource onUnresolved(final String name, final String publicId, final String baseURI, final String systemId) throws SAXException, IOException { - throw new SAXException(HardeningException.forbidden(name, null, publicId, systemId, baseURI)); + return new InputSource(new ByteArrayInputStream(new byte[0])); } private InputSource resolveWithDelegate(final String name, final String publicId, final String baseURI, diff --git a/src/main/java/org/apache/commons/xml/FallbackDenyLSResourceResolver.java b/src/main/java/org/apache/commons/xml/FallbackIgnoreLSResourceResolver.java similarity index 59% rename from src/main/java/org/apache/commons/xml/FallbackDenyLSResourceResolver.java rename to src/main/java/org/apache/commons/xml/FallbackIgnoreLSResourceResolver.java index 60ab7b5..ab2c030 100644 --- a/src/main/java/org/apache/commons/xml/FallbackDenyLSResourceResolver.java +++ b/src/main/java/org/apache/commons/xml/FallbackIgnoreLSResourceResolver.java @@ -17,25 +17,38 @@ package org.apache.commons.xml; +import org.w3c.dom.bootstrap.DOMImplementationRegistry; +import org.w3c.dom.ls.DOMImplementationLS; import org.w3c.dom.ls.LSInput; import org.w3c.dom.ls.LSResourceResolver; /** - * {@link LSResourceResolver} floor: consults an optional caller-supplied resolver and denies (throws) whatever the caller does not resolve. + * {@link LSResourceResolver} floor: consults an optional caller-supplied resolver and ignores (resolves to empty) whatever the caller does not resolve. * - * <p>The schema-compile counterpart of {@link FallbackDenyEntityResolver2}. The hardened {@link javax.xml.validation.SchemaFactory}, {@link + * <p>The schema-compile counterpart of {@link FallbackIgnoreEntityResolver2}. The hardened {@link javax.xml.validation.SchemaFactory}, {@link * javax.xml.validation.Validator} and {@link javax.xml.validation.ValidatorHandler} wrappers install one of these and route a caller-set resolver through * {@link #setDelegate} rather than letting it replace the floor. A caller opts a specific resource in by returning a non-{@code null} {@link LSInput}; - * anything left unresolved is denied.</p> + * anything left unresolved resolves to an empty {@link LSInput}, so the external resource is neither fetched nor leaked.</p> */ -final class FallbackDenyLSResourceResolver implements LSResourceResolver { +final class FallbackIgnoreLSResourceResolver implements LSResourceResolver { + + /** DOM Level 3 Load/Save implementation used to build the empty input for unresolved lookups. */ + private static final DOMImplementationLS DOM_LS = domImplementationLS(); private LSResourceResolver delegate; - FallbackDenyLSResourceResolver(final LSResourceResolver delegate) { + FallbackIgnoreLSResourceResolver(final LSResourceResolver delegate) { this.delegate = delegate; } + private static DOMImplementationLS domImplementationLS() { + try { + return (DOMImplementationLS) DOMImplementationRegistry.newInstance().getDOMImplementation("LS"); + } catch (final ClassNotFoundException | InstantiationException | IllegalAccessException e) { + throw new HardeningException("No DOM Level 3 Load/Save implementation available to build the empty schema input", e); + } + } + void setDelegate(final LSResourceResolver delegate) { this.delegate = delegate; } @@ -50,6 +63,8 @@ public LSInput resolveResource(final String type, final String namespaceURI, fin if (resolved != null) { return resolved; } - throw new SecurityException(HardeningException.forbidden(type, namespaceURI, publicId, systemId, baseURI)); + final LSInput empty = DOM_LS.createLSInput(); + empty.setStringData(""); + return empty; } } diff --git a/src/main/java/org/apache/commons/xml/FallbackDenyURIResolver.java b/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java similarity index 72% rename from src/main/java/org/apache/commons/xml/FallbackDenyURIResolver.java rename to src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java index 8f5acfa..bd2981a 100644 --- a/src/main/java/org/apache/commons/xml/FallbackDenyURIResolver.java +++ b/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java @@ -17,23 +17,26 @@ package org.apache.commons.xml; +import java.io.StringReader; + import javax.xml.transform.Source; import javax.xml.transform.TransformerException; import javax.xml.transform.URIResolver; +import javax.xml.transform.stream.StreamSource; /** - * {@link URIResolver} floor: consults an optional caller-supplied resolver and denies (throws) whatever the caller does not resolve. + * {@link URIResolver} floor: consults an optional caller-supplied resolver and ignores (resolves to empty) whatever the caller does not resolve. * - * <p>The XSLT counterpart of {@link FallbackDenyEntityResolver2}, guarding {@code xsl:import}/{@code xsl:include} at compile time and {@code document()} at + * <p>The XSLT counterpart of {@link FallbackIgnoreEntityResolver2}, guarding {@code xsl:import}/{@code xsl:include} at compile time and {@code document()} at * transform time. The hardened {@link javax.xml.transform.TransformerFactory} and {@link javax.xml.transform.Transformer} wrappers install one of these and * route a caller-set resolver through {@link #setDelegate} rather than letting it replace the floor. A caller opts a specific URI in by returning a - * non-{@code null} {@link Source}; anything left unresolved is denied.</p> + * non-{@code null} {@link Source}; anything left unresolved resolves to an empty {@link Source}, so the external resource is neither fetched nor leaked.</p> */ -final class FallbackDenyURIResolver implements URIResolver { +final class FallbackIgnoreURIResolver implements URIResolver { private URIResolver delegate; - FallbackDenyURIResolver(final URIResolver delegate) { + FallbackIgnoreURIResolver(final URIResolver delegate) { this.delegate = delegate; } @@ -48,9 +51,6 @@ URIResolver getDelegate() { @Override public Source resolve(final String href, final String base) throws TransformerException { final Source resolved = delegate != null ? delegate.resolve(href, base) : null; - if (resolved != null) { - return resolved; - } - throw new TransformerException(HardeningException.forbidden("uri", null, null, href, base)); + return resolved != null ? resolved : new StreamSource(new StringReader("")); } } diff --git a/src/main/java/org/apache/commons/xml/FallbackIgnoreXMLResolver.java b/src/main/java/org/apache/commons/xml/FallbackIgnoreXMLResolver.java index 38f1a0c..9894f19 100644 --- a/src/main/java/org/apache/commons/xml/FallbackIgnoreXMLResolver.java +++ b/src/main/java/org/apache/commons/xml/FallbackIgnoreXMLResolver.java @@ -24,11 +24,14 @@ import javax.xml.stream.XMLStreamException; /** - * {@link FallbackDenyXMLResolver} variant whose unresolved policy returns an empty input instead of throwing, so the parse continues with no replacement - * content. Used on Woodstox's DTD-subset and undeclared-entity hooks (where a missing resource must be skipped, not denied), while still consulting an - * optional caller-supplied resolver first. + * {@link XMLResolver} floor: consults an optional caller-supplied resolver and ignores (resolves to empty) whatever the caller does not resolve. + * + * <p>The StAX counterpart of {@link FallbackIgnoreEntityResolver2}, installed on each entity-resolution hook. The hardened {@link javax.xml.stream.XMLInputFactory} + * wrapper routes a caller-set resolver through {@link #setDelegate} rather than letting it replace the floor. A caller opts a specific entity in by returning + * a non-{@code null} result; anything left unresolved resolves to an empty input, so the external resource is neither fetched nor leaked and the parse + * continues with no replacement content.</p> */ -class FallbackIgnoreXMLResolver extends FallbackDenyXMLResolver { +final class FallbackIgnoreXMLResolver implements XMLResolver { /** * Empty {@link ByteArrayInputStream} shared across every call. {@code read()} on a zero-length array always returns {@code -1}, so reusing the instance @@ -36,12 +39,23 @@ class FallbackIgnoreXMLResolver extends FallbackDenyXMLResolver { */ private static final InputStream EMPTY = new ByteArrayInputStream(new byte[0]); + private XMLResolver delegate; + FallbackIgnoreXMLResolver(final XMLResolver delegate) { - super(delegate); + this.delegate = delegate; + } + + void setDelegate(final XMLResolver delegate) { + this.delegate = delegate; + } + + XMLResolver getDelegate() { + return delegate; } @Override - protected Object onUnresolved(final String publicID, final String systemID, final String baseURI, final String namespace) throws XMLStreamException { - return EMPTY; + public Object resolveEntity(final String publicID, final String systemID, final String baseURI, final String namespace) throws XMLStreamException { + final Object resolved = delegate != null ? delegate.resolveEntity(publicID, systemID, baseURI, namespace) : null; + return resolved != null ? resolved : EMPTY; } } diff --git a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilder.java b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilder.java index 44a3ef3..8343921 100644 --- a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilder.java +++ b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilder.java @@ -30,17 +30,17 @@ import org.xml.sax.SAXException; /** - * {@link DocumentBuilder} wrapper that keeps a deny-all {@link EntityResolver} as a non-overridable floor. + * {@link DocumentBuilder} wrapper that keeps an ignore-all {@link EntityResolver} as a non-overridable floor. * - * <p>A caller-set resolver is sandwiched inside a {@link FallbackDenyEntityResolver2} instead of replacing the deny-all one, so an external lookup the - * caller's resolver does not satisfy is denied rather than fetched. {@link #reset()} re-establishes the bare deny-all floor, matching the just-constructed + * <p>A caller-set resolver is sandwiched inside a {@link FallbackIgnoreEntityResolver2} instead of replacing the ignore-all one, so an external lookup the + * caller's resolver does not satisfy resolves to empty rather than being fetched. {@link #reset()} re-establishes the bare ignore-all floor, matching the just-constructed * state.</p> */ final class HardeningDocumentBuilder extends DocumentBuilder { private final DocumentBuilder delegate; - private final FallbackDenyEntityResolver2 floor = new FallbackDenyEntityResolver2(null); + private final FallbackIgnoreEntityResolver2 floor = new FallbackIgnoreEntityResolver2(null); HardeningDocumentBuilder(final DocumentBuilder delegate) { this.delegate = delegate; diff --git a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java index 35b33bb..2ae105f 100644 --- a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java @@ -25,7 +25,7 @@ import org.xml.sax.EntityResolver; /** - * {@link DocumentBuilderFactory} wrapper that keeps a deny-all {@link EntityResolver} floor on every {@link DocumentBuilder} produced. + * {@link DocumentBuilderFactory} wrapper that keeps an ignore-all {@link EntityResolver} floor on every {@link DocumentBuilder} produced. * * <p>Wraps each produced builder in a {@link HardeningDocumentBuilder}; required when the underlying factory carries no resolver of its own and does not honor * JAXP 1.5 {@code ACCESS_EXTERNAL_*} (e.g. the external Xerces distribution). A caller-set resolver is routed through the floor rather than replacing it. Kept diff --git a/src/main/java/org/apache/commons/xml/HardeningException.java b/src/main/java/org/apache/commons/xml/HardeningException.java index b980162..a7429cc 100644 --- a/src/main/java/org/apache/commons/xml/HardeningException.java +++ b/src/main/java/org/apache/commons/xml/HardeningException.java @@ -51,19 +51,4 @@ class HardeningException extends IllegalStateException { static HardeningException settingFailed(final String kind, final String name, final Object target, final Throwable cause) { return new HardeningException("Failed to set " + kind + " '" + name + "' on " + target.getClass().getName(), cause); } - - /** - * Builds the standard "forbidden by hardening" message shared by every resolver floor. - * - * @param type the resource kind, or {@code null} if not applicable. - * @param namespace the namespace (or, for Woodstox, the entity name), or {@code null}. - * @param publicId the public identifier, or {@code null} if none. - * @param systemId the system identifier of the denied resource. - * @param baseURI the base URI for relative resolution, or {@code null}. - * @return the message describing the denied lookup. - */ - static String forbidden(final String type, final String namespace, final String publicId, final String systemId, final String baseURI) { - return String.format("External resource fetch forbidden by hardening: type=%s, namespace=%s, publicId=%s, systemId=%s, baseURI=%s", type, namespace, - publicId, systemId, baseURI); - } } diff --git a/src/main/java/org/apache/commons/xml/HardeningSchema.java b/src/main/java/org/apache/commons/xml/HardeningSchema.java index 3b19c1e..87244f9 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchema.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchema.java @@ -24,7 +24,7 @@ /** * {@link Schema} wrapper that hardens every {@link Validator} and {@link ValidatorHandler} the inner Schema produces: each {@link Validator} is wrapped in * {@link HardeningValidator} (which rewrites the Source through {@link XmlFactories#harden(javax.xml.transform.Source)} and installs the resolver floor), and - * each {@link ValidatorHandler} is wrapped in a {@link HardeningValidatorHandler} that keeps the same deny-all resolver floor so {@code xsi:schemaLocation} is + * each {@link ValidatorHandler} is wrapped in a {@link HardeningValidatorHandler} that keeps the same ignore-all resolver floor so {@code xsi:schemaLocation} is * not resolved during SAX-driven validation. */ final class HardeningSchema extends Schema { diff --git a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java index a8f4df2..14c28c1 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java @@ -36,7 +36,7 @@ * * <p>Three layers cooperate:</p> * <ol> - * <li>{@link HardeningSchemaFactory} installs a deny-all {@link FallbackDenyLSResourceResolver} floor on the factory (blocking + * <li>{@link HardeningSchemaFactory} installs an ignore-all {@link FallbackIgnoreLSResourceResolver} floor on the factory (blocking * {@code xs:import}/{@code xs:include}/{@code xs:redefine} at compile time) and rewrites the Source on every {@code newSchema(Source[])} entry point * through {@link XmlFactories#harden(Source)}.</li> * <li>{@link HardeningSchema} wraps every Validator/ValidatorHandler the inner Schema produces and re-installs the floor on each (blocking @@ -55,7 +55,7 @@ final class HardeningSchemaFactory extends SchemaFactory { private final SchemaFactory delegate; - private final FallbackDenyLSResourceResolver floor = new FallbackDenyLSResourceResolver(null); + private final FallbackIgnoreLSResourceResolver floor = new FallbackIgnoreLSResourceResolver(null); HardeningSchemaFactory(final SchemaFactory delegate) { this.delegate = delegate; @@ -65,7 +65,7 @@ final class HardeningSchemaFactory extends SchemaFactory { @Override public void setResourceResolver(final LSResourceResolver resourceResolver) { - // Route a caller resolver through the floor instead of replacing it, so the deny-all lower bound cannot be removed. + // Route a caller resolver through the floor instead of replacing it, so the ignore-all lower bound cannot be removed. floor.setDelegate(resourceResolver); } diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformer.java b/src/main/java/org/apache/commons/xml/HardeningTransformer.java index d8a1b2e..a1b9322 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformer.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformer.java @@ -29,8 +29,8 @@ /** * {@link Transformer} wrapper that rewrites the Source on every {@link Transformer#transform(Source, Result)} call through - * {@link XmlFactories#harden(Source)} before delegating, and keeps a deny-all {@link URIResolver} floor so runtime {@code document()} calls a caller does not - * resolve are denied rather than fetched. + * {@link XmlFactories#harden(Source)} before delegating, and keeps an ignore-all {@link URIResolver} floor so runtime {@code document()} calls a caller does not + * resolve return empty rather than being fetched. * * <p>The floor is installed on the delegate transformer at construction, seeded with the factory's compile-time resolver; {@link #setURIResolver(URIResolver)} * routes a caller's resolver through it rather than replacing it, so the block cannot be dropped.</p> @@ -39,11 +39,11 @@ final class HardeningTransformer extends Transformer { private final Transformer delegate; - private final FallbackDenyURIResolver floor; + private final FallbackIgnoreURIResolver floor; HardeningTransformer(final Transformer delegate, final URIResolver uriResolver) { this.delegate = delegate; - this.floor = new FallbackDenyURIResolver(uriResolver); + this.floor = new FallbackIgnoreURIResolver(uriResolver); delegate.setURIResolver(floor); } diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java index c6146dc..ed8ac0f 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java @@ -60,7 +60,7 @@ final class HardeningTransformerFactory extends SAXTransformerFactory { private final SAXTransformerFactory delegate; - private final FallbackDenyURIResolver floor = new FallbackDenyURIResolver(null); + private final FallbackIgnoreURIResolver floor = new FallbackIgnoreURIResolver(null); HardeningTransformerFactory(final SAXTransformerFactory delegate) { this.delegate = delegate; diff --git a/src/main/java/org/apache/commons/xml/HardeningValidator.java b/src/main/java/org/apache/commons/xml/HardeningValidator.java index 47fd6e5..cb30279 100644 --- a/src/main/java/org/apache/commons/xml/HardeningValidator.java +++ b/src/main/java/org/apache/commons/xml/HardeningValidator.java @@ -32,19 +32,19 @@ /** * {@link Validator} wrapper that rewrites the Source on every {@link Validator#validate(Source)} and {@link Validator#validate(Source, Result)} call through - * {@link XmlFactories#harden(Source)} before delegating, and keeps a deny-all {@link LSResourceResolver} floor so {@code xsi:schemaLocation} is not resolved at + * {@link XmlFactories#harden(Source)} before delegating, and keeps an ignore-all {@link LSResourceResolver} floor so {@code xsi:schemaLocation} is not resolved at * validation time. */ final class HardeningValidator extends Validator { private final Validator delegate; - private final FallbackDenyLSResourceResolver floor = new FallbackDenyLSResourceResolver(null); + private final FallbackIgnoreLSResourceResolver floor = new FallbackIgnoreLSResourceResolver(null); HardeningValidator(final Validator delegate) { this.delegate = delegate; // Block xsi:schemaLocation resolution; neither the JDK nor Xerces reliably propagates the factory's resolver to its Validators. The floor is a - // non-removable lower bound: a caller opts specific lookups in by setting their own resolver, but cannot drop the deny-all block. + // non-removable lower bound: a caller opts specific lookups in by setting their own resolver, but cannot drop the ignore-all block. delegate.setResourceResolver(floor); } @@ -90,7 +90,7 @@ public void setProperty(final String name, final Object object) throws SAXNotRec @Override public void setResourceResolver(final LSResourceResolver resourceResolver) { - // Route a caller resolver through the floor instead of replacing it, so the deny-all lower bound cannot be removed. + // Route a caller resolver through the floor instead of replacing it, so the ignore-all lower bound cannot be removed. floor.setDelegate(resourceResolver); } diff --git a/src/main/java/org/apache/commons/xml/HardeningValidatorHandler.java b/src/main/java/org/apache/commons/xml/HardeningValidatorHandler.java index f06c418..ea25b5b 100644 --- a/src/main/java/org/apache/commons/xml/HardeningValidatorHandler.java +++ b/src/main/java/org/apache/commons/xml/HardeningValidatorHandler.java @@ -30,16 +30,16 @@ import org.xml.sax.SAXNotSupportedException; /** - * {@link ValidatorHandler} wrapper that keeps a deny-all {@link LSResourceResolver} floor a caller cannot remove. + * {@link ValidatorHandler} wrapper that keeps an ignore-all {@link LSResourceResolver} floor a caller cannot remove. * * <p>Blocks {@code xsi:schemaLocation} resolution during SAX-driven validation. A caller-set resolver is routed through a {@link - * FallbackDenyLSResourceResolver} rather than replacing the floor, so a schema the caller does not resolve is denied instead of fetched.</p> + * FallbackIgnoreLSResourceResolver} rather than replacing the floor, so a schema the caller does not resolve resolves to empty instead of being fetched.</p> */ final class HardeningValidatorHandler extends ValidatorHandler { private final ValidatorHandler delegate; - private final FallbackDenyLSResourceResolver floor = new FallbackDenyLSResourceResolver(null); + private final FallbackIgnoreLSResourceResolver floor = new FallbackIgnoreLSResourceResolver(null); HardeningValidatorHandler(final ValidatorHandler delegate) { this.delegate = delegate; diff --git a/src/main/java/org/apache/commons/xml/HardeningXMLInputFactory.java b/src/main/java/org/apache/commons/xml/HardeningXMLInputFactory.java index 66eee4d..87b8b9d 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXMLInputFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningXMLInputFactory.java @@ -32,13 +32,18 @@ import javax.xml.transform.Source; /** - * {@link XMLInputFactory} wrapper that keeps the {@link FallbackDenyXMLResolver} floors {@link StaxHardener} installs on the entity-resolution hooks + * {@link XMLInputFactory} wrapper that installs a non-removable {@link FallbackIgnoreXMLResolver} floor on the delegate's entity-resolution hook and keeps it * non-removable by the caller. * + * <p>The constructor installs the floor through {@code setXMLResolver}, which every implementation routes external resolution through (Woodstox fans it out to + * both its DTD-subset and entity resolvers). Woodstox keeps one hook outside that fan-out, {@value StaxHardener#WSTX_UNDECLARED_ENTITY_RESOLVER}, so a second + * floor is installed there best-effort: emptying the external subset leaves any entity it declared undeclared, and without that floor Woodstox rejects the + * reference instead of resolving it to empty like every other implementation.</p> + * * <p>Every resolver-valued entry point ({@link #setXMLResolver(XMLResolver)}, {@code setProperty(XMLInputFactory.RESOLVER, ...)} and the Woodstox - * {@code com.ctc.wstx.*Resolver} keys) is routed uniformly: a caller who supplies their own {@link FallbackDenyXMLResolver} takes control and it is + * {@code com.ctc.wstx.*Resolver} keys) is routed uniformly: a caller who supplies their own {@link FallbackIgnoreXMLResolver} takes control and it is * passed straight to the delegate; otherwise the current resolver on that hook is read, and if it is one of our floors the caller's resolver is set as its - * {@link FallbackDenyXMLResolver#setDelegate delegate} (an opt-in the floor cannot be removed by), or, if the hook is empty, the caller's resolver is + * {@link FallbackIgnoreXMLResolver#setDelegate delegate} (an opt-in the floor cannot be removed by), or, if the hook is empty, the caller's resolver is * wrapped in a fresh floor. This matters because Woodstox does not chain resolvers: when a resolver returns {@code null}, {@code DefaultInputResolver} falls * through to fetching the systemId URL itself, so a caller-set resolver that returns {@code null} must still land behind the floor. {@link #getXMLResolver()} and * {@code getProperty} report the caller's resolver unwrapped.</p> @@ -49,6 +54,13 @@ final class HardeningXMLInputFactory extends XMLInputFactory { HardeningXMLInputFactory(final XMLInputFactory delegate) { this.delegate = delegate; + delegate.setXMLResolver(new FallbackIgnoreXMLResolver(null)); + try { + // Woodstox only: the undeclared-entity hook is not covered by setXMLResolver, and an entity declared by the now-emptied external subset arrives here. + delegate.setProperty(StaxHardener.WSTX_UNDECLARED_ENTITY_RESOLVER, new FallbackIgnoreXMLResolver(null)); + } catch (final Exception e) { + // Not recognized by this implementation (the JDK Zephyr); nothing to install. + } } @Override @@ -83,18 +95,18 @@ public Object getProperty(final String name) { * Routes a caller-set resolver for the property {@code name} behind the floor currently installed on that hook. * * @param name The resolver-valued property being set. - * @param resolver The caller's resolver, or their own {@link FallbackDenyXMLResolver} to take control. + * @param resolver The caller's resolver, or their own {@link FallbackIgnoreXMLResolver} to take control. */ private void setResolverProperty(final String name, final XMLResolver resolver) { - if (resolver instanceof FallbackDenyXMLResolver) { + if (resolver instanceof FallbackIgnoreXMLResolver) { // The caller supplies their own floor: hand it to the delegate as-is. delegate.setProperty(name, resolver); } else { final Object current = delegate.getProperty(name); - if (current instanceof FallbackDenyXMLResolver) { - ((FallbackDenyXMLResolver) current).setDelegate(resolver); + if (current instanceof FallbackIgnoreXMLResolver) { + ((FallbackIgnoreXMLResolver) current).setDelegate(resolver); } else { - delegate.setProperty(name, new FallbackDenyXMLResolver(resolver)); + delegate.setProperty(name, new FallbackIgnoreXMLResolver(resolver)); } } } @@ -107,7 +119,7 @@ private static boolean isResolverProperty(final String name) { } private static XMLResolver unwrap(final XMLResolver resolver) { - return resolver instanceof FallbackDenyXMLResolver ? ((FallbackDenyXMLResolver) resolver).getDelegate() : resolver; + return resolver instanceof FallbackIgnoreXMLResolver ? ((FallbackIgnoreXMLResolver) resolver).getDelegate() : resolver; } // <editor-fold defaultstate="collapsed" desc="Trivial delegation"> diff --git a/src/main/java/org/apache/commons/xml/HardeningXMLReader.java b/src/main/java/org/apache/commons/xml/HardeningXMLReader.java index 666ccf2..7621571 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXMLReader.java +++ b/src/main/java/org/apache/commons/xml/HardeningXMLReader.java @@ -30,30 +30,24 @@ import org.xml.sax.XMLReader; /** - * {@link XMLReader} wrapper that keeps a {@link FallbackDenyEntityResolver2} floor as the reader's entity resolver, non-overridable by the caller. + * {@link XMLReader} wrapper that keeps a {@link FallbackIgnoreEntityResolver2} floor as the reader's entity resolver, non-overridable by the caller. * * <p>The floor is installed once and stays the reader's entity resolver for the wrapper's lifetime; {@link #setEntityResolver(EntityResolver)} routes the - * caller's resolver through {@link FallbackDenyEntityResolver2#setDelegate} instead of replacing it. This includes the {@code DefaultHandler} that + * caller's resolver through {@link FallbackIgnoreEntityResolver2#setDelegate} instead of replacing it. This includes the {@code DefaultHandler} that * {@link javax.xml.parsers.SAXParser#parse(org.xml.sax.InputSource, org.xml.sax.helpers.DefaultHandler) SAXParser.parse(source, handler)} installs as the * reader's entity resolver, which would otherwise silently replace the floor. {@link #getEntityResolver()} reports the caller's resolver unwrapped.</p> * - * <p>A path that needs a non-deny floor (e.g. one that also permits the external DTD subset) passes a {@link FallbackDenyEntityResolver2} subclass to the - * two-argument constructor; a single stable floor instance also lets that subclass double as a {@link org.xml.sax.ext.LexicalHandler}. Every other method - * forwards to the wrapped delegate; subclasses (e.g. {@code HardeningExpatXMLReader}) add per-implementation fixups on top of the floor.</p> + * <p>Every other method forwards to the wrapped delegate; subclasses (e.g. {@code HardeningExpatXMLReader}) add per-implementation fixups on top of the floor.</p> */ class HardeningXMLReader implements XMLReader { private final XMLReader delegate; - private final FallbackDenyEntityResolver2 floor; + private final FallbackIgnoreEntityResolver2 floor; HardeningXMLReader(final XMLReader delegate) { - this(delegate, new FallbackDenyEntityResolver2(null)); - } - - HardeningXMLReader(final XMLReader delegate, final FallbackDenyEntityResolver2 floor) { this.delegate = delegate; - this.floor = floor; + this.floor = new FallbackIgnoreEntityResolver2(null); delegate.setEntityResolver(floor); } diff --git a/src/main/java/org/apache/commons/xml/SAXParserHardener.java b/src/main/java/org/apache/commons/xml/SAXParserHardener.java index 272449e..b609334 100644 --- a/src/main/java/org/apache/commons/xml/SAXParserHardener.java +++ b/src/main/java/org/apache/commons/xml/SAXParserHardener.java @@ -17,9 +17,6 @@ package org.apache.commons.xml; -import java.io.IOException; -import java.util.Objects; - import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParserFactory; @@ -34,7 +31,6 @@ import org.xml.sax.SAXNotRecognizedException; import org.xml.sax.SAXNotSupportedException; import org.xml.sax.XMLReader; -import org.xml.sax.ext.LexicalHandler; /** * Capability-driven hardening for any {@link SAXParserFactory} on the classpath. @@ -44,15 +40,15 @@ * funnelled through {@link HardeningSAXParserFactory} into {@link #hardenReader(XMLReader)}:</p> * <ul> * <li><strong>Android</strong> (Harmony / Expat): {@link XMLConstants#FEATURE_SECURE_PROCESSING FSP} and the JAXP 1.5 {@code ACCESS_EXTERNAL_*} properties - * are not recognized, and libexpat enforces its own Billion Laughs check, so neither is applied. Two fixups are still needed: a subset-aware deny-all - * resolver (Expat ignores external fetches silently when no resolver is set, so an explicit one is required to <em>fail</em> on external entities while - * still letting an unused external subset load), and a {@link HardeningExpatXMLReader} so the unsupported {@code namespace-prefixes} feature is rejected at + * are not recognized, and libexpat enforces its own Billion Laughs check, so neither is applied. Two fixups are still needed: an ignore-all resolver + * (Expat ignores external fetches silently when no resolver is set; the floor keeps that behavior non-bypassable, resolving anything unresolved to + * empty), and a {@link HardeningExpatXMLReader} so the unsupported {@code namespace-prefixes} feature is rejected at * configuration time rather than mid-parse.</li> * <li><strong>FSP</strong>: required on every other reader. It switches on the implementation's built-in security manager, which is what carries the * processing limits.</li> * <li><strong>{@code XERCES_LOAD_EXTERNAL_DTD}</strong>: optional. Where supported, it skips the external DTD subset on non-validating parsers so a - * DOCTYPE-only document parses without a fetch attempt. If not supported, the fetch will throw instead, due to the following settings.</li> - * <li><strong>Deny-all resolver floor</strong>: every reader is wrapped in a {@link HardeningXMLReader} that keeps a deny-all {@link EntityResolver} floor. + * DOCTYPE-only document parses without a fetch attempt. If not supported, the resolver floor below resolves the subset to empty instead.</li> + * <li><strong>Ignore-all resolver floor</strong>: every reader is wrapped in a {@link HardeningXMLReader} that keeps an ignore-all {@link EntityResolver} floor. * That floor blocks external DTD, entity, schema and {@code xi:include} fetches in one place: the stock JDK's XInclude processor ignores * {@code ACCESS_EXTERNAL_*} and consults the {@link EntityResolver} instead, so no {@code ACCESS_EXTERNAL_*} properties are needed here. A caller can * chain its own resolver onto the floor to allow-list resources, but cannot remove it.</li> @@ -60,49 +56,6 @@ */ final class SAXParserHardener { - /** - * Deny floor that additionally lets the external DTD subset declared by the DOCTYPE be skipped silently; merely <em>declaring</em> an external subset does - * not throw. - * - * <p>Android's Expat routes every external fetch (subset, DOCTYPE {@code SYSTEM}, general/parameter entity) through the 2-arg - * {@link EntityResolver#resolveEntity(String, String)}; a deny-all resolver there would also reject a DOCTYPE that merely <em>names</em> an unused external - * subset. As a {@link FallbackDenyEntityResolver2} it consults the caller's resolver first; as a {@link LexicalHandler} (via {@code DefaultHandler2}) it - * tracks the declared subset's identifiers so {@link #onUnresolved} can tell the subset apart from a forbidden external general or parameter entity. It is - * stateful, so a fresh instance is installed per reader.</p> - */ - private static final class DtdAwareDenyResolver extends FallbackDenyEntityResolver2 { - - private String dtdPublicId; - private String dtdSystemId; - private boolean inDtd; - - DtdAwareDenyResolver() { - super(null); - } - - @Override - public void startDTD(final String name, final String publicId, final String systemId) { - inDtd = true; - dtdPublicId = publicId; - dtdSystemId = systemId; - } - - @Override - public void endDTD() { - inDtd = false; - } - - @Override - protected InputSource onUnresolved(final String name, final String publicId, final String baseURI, final String systemId) - throws SAXException, IOException { - // Declaring (but not using) an external subset must not throw: let the parser skip it silently. Everything else is denied by the floor. - if (inDtd && Objects.equals(publicId, dtdPublicId) && Objects.equals(systemId, dtdSystemId)) { - return null; - } - return super.onUnresolved(name, publicId, baseURI, systemId); - } - } - /** * {@link HardeningXMLReader} for Android's {@code org.apache.harmony.xml.ExpatReader} that additionally surfaces its {@code namespace-prefixes} limitation at * configuration time. @@ -115,8 +68,8 @@ static final class HardeningExpatXMLReader extends HardeningXMLReader { private static final String NAMESPACE_PREFIXES_FEATURE = "http://xml.org/sax/features/namespace-prefixes";; - HardeningExpatXMLReader(final XMLReader delegate, final FallbackDenyEntityResolver2 floor) { - super(delegate, floor); + HardeningExpatXMLReader(final XMLReader delegate) { + super(delegate); } @Override @@ -134,9 +87,6 @@ public void setFeature(final String name, final boolean value) throws SAXNotReco /** Class name of Android's Expat-backed {@link XMLReader}. */ private static final String ANDROID_EXPAT_READER = "org.apache.harmony.xml.ExpatReader"; - /** SAX property carrying the {@link LexicalHandler}; used to observe the DTD boundary on Android's Expat. */ - private static final String LEXICAL_HANDLER_PROPERTY = "http://xml.org/sax/properties/lexical-handler";; - /** Xerces feature: load the external DTD subset for non-validating parsers. */ private static final String XERCES_LOAD_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd";; @@ -162,20 +112,16 @@ static XMLReader hardenReader(final XMLReader reader) { return reader; } if (ANDROID_EXPAT_READER.equals(reader.getClass().getName())) { - // Expat ignores external fetches when no resolver is set; a subset-aware deny floor fails on external entities but lets an unused subset load. - // HardeningExpatXMLReader keeps that floor non-bypassable (routing a caller-set resolver, including SAXParser.parse's handler, through it) and rejects - // the unsupported namespace-prefixes feature eagerly rather than mid-parse. - final DtdAwareDenyResolver floor = new DtdAwareDenyResolver(); - final HardeningExpatXMLReader hardened = new HardeningExpatXMLReader(reader, floor); - // The floor needs the DTD-boundary events to tell the subset apart from entities; Expat recognizes the lexical-handler property. - trySetProperty(hardened, LEXICAL_HANDLER_PROPERTY, floor); - return hardened; + // Expat ignores external fetches when no resolver is set; the ignore-all floor keeps that behavior non-bypassable (routing a caller-set resolver, + // including SAXParser.parse's handler, through it and resolving anything unresolved to empty) and, via HardeningExpatXMLReader, rejects the + // unsupported namespace-prefixes feature eagerly rather than mid-parse. + return new HardeningExpatXMLReader(reader); } // Required: enables the JDK XMLSecurityManager / Xerces SecurityManager limits. setFeature(reader, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Optional: skip the external DTD subset on non-validating parsers so DOCTYPE-only documents parse without a blocked fetch attempt. setOptionalFeature(reader, XERCES_LOAD_EXTERNAL_DTD, false); - // Required: HardeningXMLReader installs a deny-all EntityResolver floor on the reader. + // Required: HardeningXMLReader installs an ignore-all EntityResolver floor on the reader. // That floor blocks external DTD, entity, schema and xi:include fetches in one place: no ACCESS_EXTERNAL_* properties are needed here. // Callers can chain their resolvers, but not override the floor. return new HardeningXMLReader(reader); @@ -230,15 +176,6 @@ private static void setOptionalFeature(final XMLReader reader, final String feat } } - private static boolean trySetProperty(final XMLReader reader, final String property, final Object value) { - try { - reader.setProperty(property, value); - return true; - } catch (final Exception e) { - return false; - } - } - private SAXParserHardener() { } } diff --git a/src/main/java/org/apache/commons/xml/SchemaHardener.java b/src/main/java/org/apache/commons/xml/SchemaHardener.java index b836414..af9cae4 100644 --- a/src/main/java/org/apache/commons/xml/SchemaHardener.java +++ b/src/main/java/org/apache/commons/xml/SchemaHardener.java @@ -23,7 +23,7 @@ * Hardening for any {@link SchemaFactory} on the classpath. * * <p>Unlike the other hardeners there is no per-implementation branching and no feature or limit configuration on the factory itself: schema compilation and - * validation reach external resources only through the resolver hook, so wrapping the factory with a non-removable deny-all resolver floor is enough on every + * validation reach external resources only through the resolver hook, so wrapping the factory with a non-removable ignore-all resolver floor is enough on every * implementation. The reader used to parse schema and instance documents is hardened separately, through * {@link SAXParserHardener#hardenSource(javax.xml.transform.Source)}.</p> */ diff --git a/src/main/java/org/apache/commons/xml/StaxHardener.java b/src/main/java/org/apache/commons/xml/StaxHardener.java index 57739d6..cdfcf9f 100644 --- a/src/main/java/org/apache/commons/xml/StaxHardener.java +++ b/src/main/java/org/apache/commons/xml/StaxHardener.java @@ -18,24 +18,13 @@ package org.apache.commons.xml; import javax.xml.stream.XMLInputFactory; -import javax.xml.stream.XMLStreamException; /** * Capability-driven hardening for any {@link XMLInputFactory} (StAX) on the classpath. * - * <p>Rather than branching on the implementation class, {@link #harden(XMLInputFactory)} consolidates the JDK Zephyr and Woodstox recipes into one pass that - * probes which properties each factory accepts and adapts:</p> - * <ul> - * <li><strong>External DTD subset</strong>: skipped via Zephyr's {@value #ZEPHYR_IGNORE_EXTERNAL_DTD} (best-effort), so a DOCTYPE-only document parses - * without a fetch attempt instead of tripping the deny-all resolver below. Woodstox skips it through {@value #WSTX_DTD_RESOLVER} instead.</li> - * <li><strong>External entities</strong>: denied through a non-removable {@link FallbackDenyXMLResolver} floor on the entity-resolution hook, - * leaving the standard {@code SUPPORT_DTD} / {@code IS_SUPPORTING_EXTERNAL_ENTITIES} defaults untouched. Woodstox exposes fine-grained hooks, so when all - * three apply the factory is Woodstox: {@value #WSTX_DTD_RESOLVER} (empty external subset, but a thrown error on external parameter entities, which share - * that hook), {@value #WSTX_ENTITY_RESOLVER} (the floor, denying declared external general entities) and {@value #WSTX_UNDECLARED_ENTITY_RESOLVER} - * (silently drop undeclared references left by the skipped subset). Any factory that does not accept that trio (the JDK Zephyr, or an unrecognized - * implementation) instead gets the floor through {@code setXMLResolver}. Either way the factory is wrapped in a {@link HardeningXMLInputFactory} that - * routes a caller-set resolver through the floor rather than letting it replace the deny-all block.</li> - * </ul> + * <p>One recipe covers both the JDK Zephyr and Woodstox: {@link HardeningXMLInputFactory} installs a non-removable {@link FallbackIgnoreXMLResolver} floor on + * every entity-resolution hook, leaving the standard {@code SUPPORT_DTD} / {@code IS_SUPPORTING_EXTERNAL_ENTITIES} defaults untouched; see that wrapper's + * Javadoc for the per-implementation hook routing.</p> */ final class StaxHardener { @@ -48,59 +37,11 @@ final class StaxHardener { /** Woodstox property: resolver consulted for undeclared entity references. */ static final String WSTX_UNDECLARED_ENTITY_RESOLVER = "com.ctc.wstx.undeclaredEntityResolver"; - /** Zephyr property: skip external DTD subset loading entirely, so a DOCTYPE-only document parses without a fetch attempt. */ - private static final String ZEPHYR_IGNORE_EXTERNAL_DTD = "http://java.sun.com/xml/stream/properties/ignore-external-dtd";; - - /** - * Woodstox DTD-subset floor: a {@link FallbackIgnoreXMLResolver} that returns the empty input for the external DTD subset (its inherited policy) - * but throws on external parameter entities. - * - * <p>Woodstox calls this hook with {@code entityName == null} for the subset and {@code entityName != null} for parameter-entity expansion (that - * discriminator is the 4th {@code resolveEntity} argument; the JDK Zephyr always passes {@code null} there). Applied best-effort, ignored by implementations - * that do not recognize the property.</p> - */ - private static final class DtdSubsetFloor extends FallbackIgnoreXMLResolver { - - DtdSubsetFloor() { - super(null); - } - - @Override - protected Object onUnresolved(final String publicID, final String systemID, final String baseURI, final String entityName) throws XMLStreamException { - // External parameter entity (entityName != null): deny, reusing the standard hardening message. - if (entityName != null) { - throw denied(publicID, systemID, baseURI, entityName); - } - // Subset (entityName == null): skip it with the empty input from the ignore floor. - return super.onUnresolved(publicID, systemID, baseURI, entityName); - } - } - static XMLInputFactory harden(final XMLInputFactory factory) { - // Optional: Zephyr's StAX equivalent of XERCES_LOAD_EXTERNAL_DTD=false skips the external DTD subset entirely. - trySetProperty(factory, ZEPHYR_IGNORE_EXTERNAL_DTD, true); - - // Each hook carries its own FallbackDenyXMLResolver floor; a caller can opt specific entities in through it, but cannot remove it (see - // HardeningXMLInputFactory, which routes a caller-set resolver into the floor rather than replacing it). The DTD-subset and undeclared-entity hooks skip - // (empty input) rather than deny on an unresolved lookup, so a DOCTYPE-only document still parses. - if (!(trySetProperty(factory, WSTX_DTD_RESOLVER, new DtdSubsetFloor()) - && trySetProperty(factory, WSTX_ENTITY_RESOLVER, new FallbackDenyXMLResolver(null)) - && trySetProperty(factory, WSTX_UNDECLARED_ENTITY_RESOLVER, new FallbackIgnoreXMLResolver(null)))) { - // Fallback (JDK Zephyr or unrecognized): the single resolver carries the deny-all floor. - factory.setXMLResolver(new FallbackDenyXMLResolver(null)); - } + // HardeningXMLInputFactory installs the non-removable ignore-all resolver floor that resolves every external DTD and entity to empty content. return new HardeningXMLInputFactory(factory); } - private static boolean trySetProperty(final XMLInputFactory factory, final String property, final Object value) { - try { - factory.setProperty(property, value); - return true; - } catch (final Exception e) { - return false; - } - } - private StaxHardener() { } } diff --git a/src/main/java/org/apache/commons/xml/TransformerHardener.java b/src/main/java/org/apache/commons/xml/TransformerHardener.java index 493e5bb..8757479 100644 --- a/src/main/java/org/apache/commons/xml/TransformerHardener.java +++ b/src/main/java/org/apache/commons/xml/TransformerHardener.java @@ -41,7 +41,7 @@ * hardening surface is reachable only through a vendor API.</li> * <li><strong>FSP</strong> ({@link XMLConstants#FEATURE_SECURE_PROCESSING}): required. On XSLTC it enables the runtime evaluator limits; on Xalan it disables * reflection-based extension functions.</li> - * <li><strong>{@link FallbackDenyURIResolver} floor</strong>: required. A deny-all {@link URIResolver} floor, installed by + * <li><strong>{@link FallbackIgnoreURIResolver} floor</strong>: required. An ignore-all {@link URIResolver} floor, installed by * {@link HardeningTransformerFactory} and carried onto every produced {@link Transformer}, blocks {@code xsl:import}/{@code xsl:include} at compile time * and {@code document()} at runtime, the one channel both XSLTC and Xalan route through. A caller-set {@link URIResolver} is routed through the floor * rather than replacing it, so a caller can opt a specific URI in but cannot drop the block.</li> @@ -69,7 +69,7 @@ static TransformerFactory harden(final TransformerFactory factory) { // Required: enables secure processing (XSLTC runtime limits; Xalan's extension-function block). setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Required: source/stylesheet parsing provisions its own SAX reader otherwise; the wrapper routes every Source through a hardened one and installs the - // deny-all URIResolver floor (blocking xsl:import/include at compile time and document() at runtime) that a caller-set resolver cannot remove. + // ignore-all URIResolver floor (blocking xsl:import/include at compile time and document() at runtime) that a caller-set resolver cannot remove. return new HardeningTransformerFactory((SAXTransformerFactory) factory); } diff --git a/src/test/java/org/apache/commons/xml/AttackTestSupport.java b/src/test/java/org/apache/commons/xml/AttackTestSupport.java index 4c80d2e..66a2f3a 100644 --- a/src/test/java/org/apache/commons/xml/AttackTestSupport.java +++ b/src/test/java/org/apache/commons/xml/AttackTestSupport.java @@ -93,7 +93,7 @@ final class AttackTestSupport { /** * Test-only permissive counterpart of {@code SAXParserHardener.HardeningExpatXMLReader}: a pass-through Expat wrapper that rejects the - * {@code namespace-prefixes} feature eagerly (so a probing TrAX identity transformer falls back instead of failing the whole parse) but installs no deny-all + * {@code namespace-prefixes} feature eagerly (so a probing TrAX identity transformer falls back instead of failing the whole parse) but installs no ignore-all * resolver floor, so the unconfigured/positive controls stay permissive. */ private static final class PermissiveExpatReader extends XMLFilterImpl { @@ -214,6 +214,16 @@ static void assertDomBlocks(final String payload) { assertParseFails(() -> strictDocumentBuilder(XmlFactories.newDocumentBuilderFactory()).parse(inputSource(payload)), "DOM", SAXException.class); } + /** + * Asserts a hardened DOM parse either blocks at parse or completes without leaked content. + * + * <p>Used for an external-resource payload whose outcome differs across implementations: one that resolves the reference to empty (the ignore-all floor) does + * not leak, while one that rejects the unresolvable systemId throws instead. Both are acceptable.</p> + */ + static void assertDomBlocksOrDoesNotLeak(final String payload) { + assertNoLeakOrThrows(() -> domParseAndCaptureText(payload), "DOM", SAXException.class); + } + /** * Asserts a hardened DOM parse completes without throwing and without leaked content. * @@ -441,6 +451,13 @@ static void assertSaxBlocks(final String payload) { assertParseFails(() -> consumeXmlReader(strictXMLReader(XmlFactories.newSAXParserFactory()), payload), "SAX", SAXException.class); } + /** + * Asserts a hardened SAX parse either blocks at parse or completes without leaked content. See {@link #assertDomBlocksOrDoesNotLeak(String)}. + */ + static void assertSaxBlocksOrDoesNotLeak(final String payload) { + assertNoLeakOrThrows(() -> captureCharacters(strictXMLReader(XmlFactories.newSAXParserFactory()), payload), "SAX", SAXException.class); + } + /** * Asserts a hardened SAX parse completes without throwing and without leaked content. * @@ -469,6 +486,17 @@ static void assertSchemaBlocks(final Source xsd) { assertParseFails(() -> strictSchema(XmlFactories.newSchemaFactory(), xsd), "Schema compile", SAXException.class, SecurityException.class); } + /** + * Asserts a hardened Schema compile either blocks or completes: an unresolved import resolves to an empty schema (which may itself fail to compile) or is + * rejected outright. See {@link #assertDomBlocksOrDoesNotLeak(String)}. + */ + static void assertSchemaBlocksOrDoesNotLeak(final Source xsd) { + assertNoLeakOrThrows(() -> { + strictSchema(XmlFactories.newSchemaFactory(), xsd); + return ""; + }, "Schema compile", SAXException.class, SecurityException.class); + } + /** * Asserts a hardened Schema compilation succeeds. * @@ -500,6 +528,14 @@ static void assertStaxBlocks(final String payload) { assertParseFails(() -> consumeEventReader(XmlFactories.newXMLInputFactory(), payload), "StAX event", XMLStreamException.class); } + /** + * Asserts a hardened StAX parse (stream and event) either blocks at parse or completes without leaked content. See {@link #assertDomBlocksOrDoesNotLeak(String)}. + */ + static void assertStaxBlocksOrDoesNotLeak(final String payload) { + assertNoLeakOrThrows(() -> captureStaxStreamText(XmlFactories.newXMLInputFactory(), payload), "StAX stream", XMLStreamException.class); + assertNoLeakOrThrows(() -> captureStaxEventText(XmlFactories.newXMLInputFactory(), payload), "StAX event", XMLStreamException.class); + } + /** * Asserts a hardened StAX parse completes without throwing and without leaked content. * @@ -539,6 +575,13 @@ static void assertTemplatesBlocks(final Source xslt) { }, "Templates", TransformerException.class); } + /** + * Asserts a hardened Templates compile-and-transform either blocks or completes without leaked content. See {@link #assertDomBlocksOrDoesNotLeak(String)}. + */ + static void assertTemplatesBlocksOrDoesNotLeak(final Source xslt) { + assertNoLeakOrThrows(() -> templatesCompileAndTransform(xslt), "Templates", TransformerException.class); + } + /** * Asserts a hardened Templates compile-and-transform succeeds. * @@ -571,6 +614,13 @@ static void assertTransformerBlocks(final String payload) { "Transformer", TransformerException.class); } + /** + * Asserts a hardened identity Transformer either blocks or completes without leaked content. See {@link #assertDomBlocksOrDoesNotLeak(String)}. + */ + static void assertTransformerBlocksOrDoesNotLeak(final String payload) { + assertNoLeakOrThrows(() -> identityTransformAndCapture(payload), "Transformer", TransformerException.class); + } + /** * Asserts a hardened identity Transformer completes without throwing and without leaked content. * @@ -603,6 +653,18 @@ static void assertValidatorBlocks(final String xml) { "Validator", SAXException.class, SecurityException.class); } + /** + * Asserts a hardened Validator either blocks or completes without leaked content. The instance document's unresolvable external entity is either dropped + * (no leak) or rejected; a rejection surfaces as a SAX/security error or, where the parser attempts the unresolvable systemId directly, an + * {@link IOException}. See {@link #assertDomBlocksOrDoesNotLeak(String)}. + */ + static void assertValidatorBlocksOrDoesNotLeak(final String xml) { + assertNoLeakOrThrows(() -> { + strictValidator(strictSchema(XmlFactories.newSchemaFactory(), streamSource(BENIGN_SCHEMA))).validate(streamSource(xml)); + return ""; + }, "Validator", SAXException.class, SecurityException.class, IOException.class); + } + /** * Asserts a hardened Validator validation completes without throwing. * @@ -636,6 +698,13 @@ static void assertXmlReaderBlocks(final String payload) { assertParseFails(() -> consumeXmlReader(rawHardenedReader(), payload), "XMLReader", SAXException.class); } + /** + * Asserts a hardened-in-place XMLReader parse either blocks at parse or completes without leaked content. See {@link #assertDomBlocksOrDoesNotLeak(String)}. + */ + static void assertXmlReaderBlocksOrDoesNotLeak(final String payload) { + assertNoLeakOrThrows(() -> captureCharacters(rawHardenedReader(), payload), "XMLReader", SAXException.class); + } + /** * Asserts a hardened-in-place XMLReader parse completes without throwing and without leaked content. * diff --git a/src/test/java/org/apache/commons/xml/EntityResolverFloorTest.java b/src/test/java/org/apache/commons/xml/EntityResolverFloorTest.java index 2762c5c..8af9715 100644 --- a/src/test/java/org/apache/commons/xml/EntityResolverFloorTest.java +++ b/src/test/java/org/apache/commons/xml/EntityResolverFloorTest.java @@ -21,7 +21,6 @@ import static org.apache.commons.xml.AttackTestSupport.assertParseSucceeds; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.StringReader; @@ -56,20 +55,20 @@ import org.xml.sax.helpers.DefaultHandler; /** - * Checks that a caller-supplied resolver cannot remove the hardened deny-all floor on any factory. + * Checks that a caller-supplied resolver cannot remove the hardened ignore-all floor on any factory. * * <p>The observable contract on every hardened factory is the same: a resource the caller resolves (returns a non-null value) is allowed, but anything the - * caller does not resolve is denied instead of fetched, so a resolver that resolves nothing leaves the block in place. Most factories enforce this with a - * {@link FallbackDenyEntityResolver2}-style floor that consults the caller and denies on a {@code null} return; Saxon enforces the equivalent through its - * {@code ALLOWED_PROTOCOLS} restrictor. Every resolver channel is exercised: the SAX/DOM {@link EntityResolver}, the StAX {@link XMLResolver}, the schema - * {@link LSResourceResolver} and the XSLT {@link URIResolver}.</p> + * caller does not resolve is resolved to empty content instead of fetched, so a resolver that resolves nothing leaves the block in place. Most + * factories enforce this with a {@link FallbackIgnoreEntityResolver2}-style floor that consults the caller and returns empty on a {@code null} return; Saxon + * enforces the equivalent through its {@code ALLOWED_PROTOCOLS} restrictor. Every resolver channel is exercised: the SAX/DOM + * {@link EntityResolver}, the StAX {@link XMLResolver}, the schema {@link LSResourceResolver} and the XSLT {@link URIResolver}.</p> */ class EntityResolverFloorTest { /** systemId the allow-list resolvers permit (its content carries {@link AttackTestSupport#LEAKED_MARKER}). */ private static final String ALLOWED = AttackTestSupport.resourceUrl("referenced.txt").toString(); - /** systemId the allow-list resolvers do not resolve (and the floor must deny). */ + /** systemId the allow-list resolvers do not resolve (so the floor resolves it to empty; its content carries {@link AttackTestSupport#LEAKED_MARKER}). */ private static final String UNLISTED = AttackTestSupport.resourceUrl("referenced.xml").toString(); // ---- Entity channel (DOM / SAX) ---------------------------------------------------------------------------------------------------------------------- @@ -109,11 +108,18 @@ void domResolvesAllowListed() throws Exception { @Test @Tag("dom") - void domDeniesUnlisted() throws Exception { + void domDoesNotLeakUnlisted() throws Exception { Assumptions.assumeTrue(AttackTestSupport.DOM_RESOLVES_INTERNAL_ENTITIES, "platform DOM does not resolve user-defined entities"); final DocumentBuilder builder = hardenedBuilder(); builder.setEntityResolver(ENTITY_ALLOW_LIST); - assertThrows(SAXException.class, () -> builder.parse(AttackTestSupport.inputSource(entityPayload(UNLISTED)))); + // The caller returns null for the unlisted entity, so the floor resolves it to empty rather than fetching it: the parse completes (or is rejected) + // without leaking the entity's content. + try { + final Document doc = builder.parse(AttackTestSupport.inputSource(entityPayload(UNLISTED))); + assertFalse(doc.getDocumentElement().getTextContent().contains(AttackTestSupport.LEAKED_MARKER), "unlisted external entity leaked into the DOM"); + } catch (final SAXException blocked) { + // Acceptable: the reference was rejected at parse rather than resolved to empty. + } } @Test @@ -135,18 +141,30 @@ public void characters(final char[] ch, final int start, final int length) { @Test @Tag("sax") - void saxReaderDeniesUnlisted() throws Exception { + void saxReaderDoesNotLeakUnlisted() throws Exception { final XMLReader reader = hardenedReader(); reader.setEntityResolver(ENTITY_ALLOW_LIST); - reader.setContentHandler(new DefaultHandler()); - assertThrows(SAXException.class, () -> reader.parse(AttackTestSupport.inputSource(entityPayload(UNLISTED)))); + final StringBuilder text = new StringBuilder(); + reader.setContentHandler(new DefaultHandler() { + @Override + public void characters(final char[] ch, final int start, final int length) { + text.append(ch, start, length); + } + }); + // The caller returns null for the unlisted entity, so the floor resolves it to empty rather than fetching it. + try { + reader.parse(AttackTestSupport.inputSource(entityPayload(UNLISTED))); + } catch (final SAXException blocked) { + return; // Acceptable: rejected at parse rather than resolved to empty. + } + assertFalse(text.toString().contains(AttackTestSupport.LEAKED_MARKER), "unlisted external entity leaked:\n" + text); } @Test @Tag("sax") void saxParseWithHandlerDoesNotBypass() throws Exception { // SAXParser.parse(source, handler) installs the handler as the reader's entity resolver; the handler does not resolve it (returns null), so the - // deny-all floor must still block the external entity rather than letting the parser fetch it. + // ignore-all floor must still resolve the external entity to empty rather than letting the parser fetch it. final SAXParser parser = XmlFactories.newSAXParserFactory().newSAXParser(); final StringBuilder text = new StringBuilder(); final DefaultHandler handler = new DefaultHandler() { @@ -273,19 +291,28 @@ void staxResolvesAllowListed() throws Exception { @Test @Tag("stax") - void staxDeniesUnlisted() { + void staxDoesNotLeakUnlisted() throws Exception { final XMLInputFactory factory = externalEntityStaxFactory(); factory.setXMLResolver(STAX_ALLOW_LIST); - assertThrows(XMLStreamException.class, () -> readStaxText(factory, entityPayload(UNLISTED))); + // The caller returns null for the unlisted entity, so the floor resolves it to empty rather than fetching it. + try { + assertFalse(readStaxText(factory, entityPayload(UNLISTED)).contains(AttackTestSupport.LEAKED_MARKER), "unlisted external entity leaked"); + } catch (final XMLStreamException blocked) { + // Acceptable: rejected at parse rather than resolved to empty. + } } @Test @Tag("stax") - void staxCallerCannotRemoveFloor() { - // A caller resolver that resolves nothing must not re-open external fetches: the floor still denies. + void staxCallerCannotRemoveFloor() throws Exception { + // A caller resolver that resolves nothing must not re-open external fetches: the floor still resolves the reference to empty rather than fetching it. final XMLInputFactory factory = externalEntityStaxFactory(); factory.setXMLResolver((publicID, systemID, baseURI, namespace) -> null); - assertThrows(XMLStreamException.class, () -> readStaxText(factory, entityPayload(ALLOWED))); + try { + assertFalse(readStaxText(factory, entityPayload(ALLOWED)).contains(AttackTestSupport.LEAKED_MARKER), "floor was bypassed and the entity leaked"); + } catch (final XMLStreamException blocked) { + // Acceptable: rejected at parse rather than resolved to empty. + } } @Test @@ -363,8 +390,8 @@ void transformerDeniesUnlisted() { } /** - * A hardened {@link TransformerFactory} with a re-throwing error listener. XSLTC and Xalan enforce the deny through the - * {@link FallbackDenyURIResolver} floor; Saxon enforces it through its {@code ALLOWED_PROTOCOLS} restrictor. Either way a caller-set resolver that + * A hardened {@link TransformerFactory} with a re-throwing error listener. XSLTC and Xalan enforce the block through the + * {@link FallbackIgnoreURIResolver} floor; Saxon enforces it through its {@code ALLOWED_PROTOCOLS} restrictor. Either way a caller-set resolver that * returns {@code null} cannot re-open the fetch. The strict listener is required because interpretive Xalan routes a blocked {@code xsl:import} through the * error listener and would otherwise recover and compile instead of throwing (XSLTC and Saxon throw regardless). */ diff --git a/src/test/java/org/apache/commons/xml/ExternalGeneralEntityTest.java b/src/test/java/org/apache/commons/xml/ExternalGeneralEntityTest.java index 6bcb89e..94767cd 100644 --- a/src/test/java/org/apache/commons/xml/ExternalGeneralEntityTest.java +++ b/src/test/java/org/apache/commons/xml/ExternalGeneralEntityTest.java @@ -67,49 +67,49 @@ private static String xsltPayload() { void hardenedDomBlocks() { Assumptions.assumeTrue(AttackTestSupport.DOM_RESOLVES_INTERNAL_ENTITIES, "Skipped: platform DOM does not resolve user-defined entities"); - AttackTestSupport.assertDomBlocks(xmlPayload()); + AttackTestSupport.assertDomBlocksOrDoesNotLeak(xmlPayload()); } @Test @Tag("sax") void hardenedSaxBlocks() { - AttackTestSupport.assertSaxBlocks(xmlPayload()); + AttackTestSupport.assertSaxBlocksOrDoesNotLeak(xmlPayload()); } @Test @Tag("schema") void hardenedSchemaBlocks() { - AttackTestSupport.assertSchemaBlocks(AttackTestSupport.streamSource(xsdPayload())); + AttackTestSupport.assertSchemaBlocksOrDoesNotLeak(AttackTestSupport.streamSource(xsdPayload())); } @Test @Tag("stax") void hardenedStaxBlocks() { - AttackTestSupport.assertStaxBlocks(xmlPayload()); + AttackTestSupport.assertStaxBlocksOrDoesNotLeak(xmlPayload()); } @Test @Tag("trax") void hardenedTemplatesBlocks() { - AttackTestSupport.assertTemplatesBlocks(AttackTestSupport.streamSource(xsltPayload())); + AttackTestSupport.assertTemplatesBlocksOrDoesNotLeak(AttackTestSupport.streamSource(xsltPayload())); } @Test @Tag("trax") void hardenedTransformerBlocks() { - AttackTestSupport.assertTransformerBlocks(xmlPayload()); + AttackTestSupport.assertTransformerBlocksOrDoesNotLeak(xmlPayload()); } @Test @Tag("schema") void hardenedValidatorBlocks() { - AttackTestSupport.assertValidatorBlocks(xmlPayload()); + AttackTestSupport.assertValidatorBlocksOrDoesNotLeak(xmlPayload()); } @Test @Tag("sax") void hardenedXmlReaderBlocks() { - AttackTestSupport.assertXmlReaderBlocks(xmlPayload()); + AttackTestSupport.assertXmlReaderBlocksOrDoesNotLeak(xmlPayload()); } @Test diff --git a/src/test/java/org/apache/commons/xml/ExternalParameterEntityTest.java b/src/test/java/org/apache/commons/xml/ExternalParameterEntityTest.java index 4b9a0de..22d562f 100644 --- a/src/test/java/org/apache/commons/xml/ExternalParameterEntityTest.java +++ b/src/test/java/org/apache/commons/xml/ExternalParameterEntityTest.java @@ -143,7 +143,7 @@ private static String xsltPayload() { void hardenedDomBlocks() { Assumptions.assumeTrue(DOM_ACCEPTS_PARAMETER_ENTITIES, "Skipped: platform DOM does not accept parameter entities"); - AttackTestSupport.assertDomBlocks(xmlPayload()); + AttackTestSupport.assertDomBlocksOrDoesNotLeak(xmlPayload()); } @Test @@ -151,7 +151,7 @@ void hardenedDomBlocks() { void hardenedSaxBlocks() { Assumptions.assumeTrue(SAX_RESOLVES_PARAMETER_ENTITIES, "Skipped: platform SAX parser does not invoke the entity resolver for parameter entities"); - AttackTestSupport.assertSaxBlocks(xmlPayload()); + AttackTestSupport.assertSaxBlocksOrDoesNotLeak(xmlPayload()); } @Test @@ -159,13 +159,13 @@ void hardenedSaxBlocks() { void hardenedSchemaBlocks() { Assumptions.assumeTrue(SAX_RESOLVES_PARAMETER_ENTITIES, "Skipped: platform SAX parser does not invoke the entity resolver for parameter entities"); - AttackTestSupport.assertSchemaBlocks(AttackTestSupport.streamSource(xsdPayload())); + AttackTestSupport.assertSchemaBlocksOrDoesNotLeak(AttackTestSupport.streamSource(xsdPayload())); } @Test @Tag("stax") void hardenedStaxBlocks() { - AttackTestSupport.assertStaxBlocks(xmlPayload()); + AttackTestSupport.assertStaxBlocksOrDoesNotLeak(xmlPayload()); } @Test @@ -173,7 +173,7 @@ void hardenedStaxBlocks() { void hardenedTemplatesBlocks() { Assumptions.assumeTrue(SAX_RESOLVES_PARAMETER_ENTITIES, "Skipped: platform SAX parser does not invoke the entity resolver for parameter entities"); - AttackTestSupport.assertTemplatesBlocks(AttackTestSupport.streamSource(xsltPayload())); + AttackTestSupport.assertTemplatesBlocksOrDoesNotLeak(AttackTestSupport.streamSource(xsltPayload())); } @Test @@ -181,7 +181,7 @@ void hardenedTemplatesBlocks() { void hardenedTransformerBlocks() { Assumptions.assumeTrue(SAX_RESOLVES_PARAMETER_ENTITIES, "Skipped: platform SAX parser does not invoke the entity resolver for parameter entities"); - AttackTestSupport.assertTransformerBlocks(xmlPayload()); + AttackTestSupport.assertTransformerBlocksOrDoesNotLeak(xmlPayload()); } @Test @@ -189,7 +189,7 @@ void hardenedTransformerBlocks() { void hardenedValidatorBlocks() { Assumptions.assumeTrue(SAX_RESOLVES_PARAMETER_ENTITIES, "Skipped: platform SAX parser does not invoke the entity resolver for parameter entities"); - AttackTestSupport.assertValidatorBlocks(xmlPayload()); + AttackTestSupport.assertValidatorBlocksOrDoesNotLeak(xmlPayload()); } @Test @@ -197,7 +197,7 @@ void hardenedValidatorBlocks() { void hardenedXmlReaderBlocks() { Assumptions.assumeTrue(SAX_RESOLVES_PARAMETER_ENTITIES, "Skipped: platform SAX parser does not invoke the entity resolver for parameter entities"); - AttackTestSupport.assertXmlReaderBlocks(xmlPayload()); + AttackTestSupport.assertXmlReaderBlocksOrDoesNotLeak(xmlPayload()); } @Test diff --git a/src/test/java/org/apache/commons/xml/SchemaLocationDomTest.java b/src/test/java/org/apache/commons/xml/SchemaLocationDomTest.java index 8b873e3..869af34 100644 --- a/src/test/java/org/apache/commons/xml/SchemaLocationDomTest.java +++ b/src/test/java/org/apache/commons/xml/SchemaLocationDomTest.java @@ -20,8 +20,7 @@ import static org.apache.commons.xml.AttackTestSupport.LEAKED_MARKER; import static org.apache.commons.xml.AttackTestSupport.resourceUrl; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assumptions.assumeTrue; import javax.xml.XMLConstants; @@ -41,9 +40,9 @@ * {@code xsi:noNamespaceSchemaLocation} hint in the instance document. * * <p>The instance is empty {@code <root/>}; the referenced schema declares a default {@code leak} attribute carrying {@link AttackTestSupport#LEAKED_MARKER}. A - * parser that fetches the schema inlines that default into the DOM (the permissive control), while a hardened parser refuses the fetch and fails the parse. The - * attribution differs by implementation (the stock JDK reports an {@code accessExternalSchema} / {@code schema_reference} error; external Xerces' deny-all - * resolver reports a forbidden-fetch error), so the test only asserts the fetch was blocked, not how.</p> + * parser that fetches the schema inlines that default into the DOM (the permissive control), while a hardened parser resolves the schema reference to empty + * content instead. Either the empty schema makes the validating parse fail, or the parse completes but the default is never inlined; either way the marker never + * reaches the DOM.</p> * * <p>The test runs only where the implementation supports JAXP 1.2 schema-language XSD validation (the stock JDK and external Xerces do; Android does not), so * it skips on parsers without it.</p> @@ -56,18 +55,19 @@ class SchemaLocationDomTest { private static final String INSTANCE = "schema-location-instance.xml"; - /** Name of the external schema the instance points at; both block mechanisms name it in the failure message. */ - private static final String SCHEMA = "schema-location.xsd"; - @Test - void hardenedBlocksExternalSchemaFetch() { + void hardenedDoesNotFetchExternalSchema() { assumeTrue(supportsSchemaLanguage(), "parser does not support JAXP 1.2 schema-language XSD validation"); final DocumentBuilderFactory factory = enableXsdValidation(XmlFactories.newDocumentBuilderFactory()); - // The schemaLocation fetch is denied and surfaced as a fatal error rather than a silent fetch. The attribution is implementation-specific, so assert - // only that the failure names the external schema, not the mechanism (accessExternalSchema on the JDK, the deny-all resolver on external Xerces). - final SAXException thrown = assertThrows(SAXException.class, () -> parse(factory)); - assertTrue(thrown.getMessage() != null && thrown.getMessage().contains(SCHEMA), - "Block must reference the external schema, got: " + thrown.getMessage()); + // The schemaLocation reference resolves to empty rather than being fetched. Either the empty schema fails the validating parse (acceptable), or the + // parse completes but the schema's default leak attribute is never inlined. Either way the marker must not reach the DOM. + try { + final Document document = parse(factory); + assertNotEquals(LEAKED_MARKER, document.getDocumentElement().getAttribute("leak"), + "Hardened parse must not inline the external schema's default attribute."); + } catch (final Exception blocked) { + // Acceptable: the empty schema was rejected at parse time, so nothing was fetched or inlined. + } } @Test diff --git a/src/test/java/org/apache/commons/xml/SchemaLocationPropertyTest.java b/src/test/java/org/apache/commons/xml/SchemaLocationPropertyTest.java index 463175f..8fc03db 100644 --- a/src/test/java/org/apache/commons/xml/SchemaLocationPropertyTest.java +++ b/src/test/java/org/apache/commons/xml/SchemaLocationPropertyTest.java @@ -45,7 +45,7 @@ * <p>The fixtures declare the instance's root element, so a parser that fetches the schema validates the instance * cleanly and one that does not cannot. The permissive controls prove the external schema is reachable in principle, so * the hardened side throwing means the fetch was refused, not merely misconfigured. The stock JDK refuses it through - * {@code accessExternalSchema=""}; external Apache Xerces, which ignores that property, refuses it through the deny-all + * {@code accessExternalSchema=""}; external Apache Xerces, which ignores that property, refuses it through the ignore-all * entity-resolver floor.</p> * * <p>Not every parser supports these schema-validation knobs (Android's KXmlParser and Expat do not), so the whole diff --git a/src/test/java/org/apache/commons/xml/SchemaLocationSaxTest.java b/src/test/java/org/apache/commons/xml/SchemaLocationSaxTest.java index 6939817..b7317e1 100644 --- a/src/test/java/org/apache/commons/xml/SchemaLocationSaxTest.java +++ b/src/test/java/org/apache/commons/xml/SchemaLocationSaxTest.java @@ -20,8 +20,7 @@ import static org.apache.commons.xml.AttackTestSupport.LEAKED_MARKER; import static org.apache.commons.xml.AttackTestSupport.resourceUrl; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assumptions.assumeTrue; import javax.xml.XMLConstants; @@ -32,7 +31,6 @@ import org.junit.jupiter.api.Test; import org.xml.sax.Attributes; import org.xml.sax.InputSource; -import org.xml.sax.SAXException; import org.xml.sax.XMLReader; import org.xml.sax.helpers.DefaultHandler; @@ -42,9 +40,9 @@ * * <p>This is the SAX counterpart of {@link SchemaLocationDomTest}. The instance is empty {@code <root/>}; the referenced schema declares a default {@code leak} * attribute carrying {@link AttackTestSupport#LEAKED_MARKER}. A parser that fetches the schema augments the element's attributes with that default (the - * permissive control observes it in {@link DefaultHandler#startElement}), while a hardened parser refuses the fetch and fails the parse. The attribution differs - * by implementation (the stock JDK reports an {@code accessExternalSchema} / {@code schema_reference} error; external Xerces' deny-all resolver reports a - * forbidden-fetch error), so the test only asserts the fetch was blocked, not how.</p> + * permissive control observes it in {@link DefaultHandler#startElement}), while a hardened parser resolves the schema reference to empty content instead. Either + * the empty schema makes the validating parse fail, or the parse completes but the default is never augmented onto the element; either way the marker is never + * observed.</p> * * <p>The test runs only where the implementation supports JAXP 1.2 schema-language XSD validation (the stock JDK and external Xerces do; Android does not), so it * skips on parsers without it.</p> @@ -69,18 +67,19 @@ public void startElement(final String uri, final String localName, final String private static final String INSTANCE = "schema-location-instance.xml"; - /** Name of the external schema the instance points at; both block mechanisms name it in the failure message. */ - private static final String SCHEMA = "schema-location.xsd"; - @Test - void hardenedBlocksExternalSchemaFetch() throws Exception { + void hardenedDoesNotFetchExternalSchema() throws Exception { assumeTrue(supportsSchemaLanguage(), "parser does not support JAXP 1.2 schema-language XSD validation"); final SAXParser parser = newValidatingParser(XmlFactories.newSAXParserFactory()); - // The schemaLocation fetch is denied and surfaced as an error rather than a silent fetch. The attribution is implementation-specific, so assert only - // that the failure names the external schema, not the mechanism (accessExternalSchema on the JDK, the deny-all resolver on external Xerces). - final SAXException thrown = assertThrows(SAXException.class, () -> parse(parser, new DefaultHandler())); - assertTrue(thrown.getMessage() != null && thrown.getMessage().contains(SCHEMA), - "Block must reference the external schema, got: " + thrown.getMessage()); + // The schemaLocation reference resolves to empty rather than being fetched. Either the empty schema fails the validating parse (acceptable), or the + // parse completes but the schema's default leak attribute is never augmented onto the element. Either way the marker must not be observed. + final LeakCapturingHandler handler = new LeakCapturingHandler(); + try { + parse(parser, handler); + } catch (final Exception blocked) { + // Acceptable: the empty schema was rejected at parse time, so nothing was fetched or augmented. + } + assertNull(handler.leak, "Hardened parse must not augment the external schema's default attribute onto the element."); } @Test @@ -106,7 +105,7 @@ private static SAXParser newValidatingParser(final SAXParserFactory factory) thr private static void parse(final SAXParser parser, final DefaultHandler handler) throws Exception { // Drive the XMLReader directly rather than SAXParser.parse(InputSource, DefaultHandler): the latter calls reader.setEntityResolver(handler), which would - // clobber the hardened deny-all resolver that external Xerces relies on to block the schemaLocation fetch. Reuse AttackTestSupport's shared strict + // clobber the hardened ignore-all resolver that external Xerces relies on to block the schemaLocation fetch. Reuse AttackTestSupport's shared strict // reporter as the error handler so a blocked fetch surfaces as a thrown exception rather than a silent recovery. final XMLReader reader = parser.getXMLReader(); reader.setContentHandler(handler); diff --git a/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java b/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java index cc2e671..f308c43 100644 --- a/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java +++ b/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java @@ -36,6 +36,8 @@ import org.vafer.jdependency.Clazz; import org.vafer.jdependency.Clazzpath; +import javax.xml.transform.Source; + /** * Guards the shade footprint: the set of classes a consumer pulls in when they shade a single hardener entry point. * @@ -53,37 +55,36 @@ class ShadingFootprintTest { private static final String PKG = "org.apache.commons.xml."; /** - * Every hardener needs this shared exception (its {@code settingFailed}/{@code forbidden} message helpers). + * Shared exception carrying the {@code settingFailed} message helper; pulled in by every hardener that applies a JAXP setting. */ private static final String HARDENING_EXCEPTION = "HardeningException"; private static final Set<String> DOCUMENT_BUILDER_HARDENER = set("DocumentBuilderHardener", "HardeningDocumentBuilder", "HardeningDocumentBuilderFactory" - , HARDENING_EXCEPTION, "FallbackDenyEntityResolver2"); + , HARDENING_EXCEPTION, "FallbackIgnoreEntityResolver2"); - private static final Set<String> SAX_PARSER_HARDENER = set("SAXParserHardener", "SAXParserHardener$DtdAwareDenyResolver", + private static final Set<String> SAX_PARSER_HARDENER = set("SAXParserHardener", "SAXParserHardener$HardeningExpatXMLReader", "HardeningSAXParser", "HardeningSAXParserFactory", "HardeningXMLReader", HARDENING_EXCEPTION, - "FallbackDenyEntityResolver2"); + "FallbackIgnoreEntityResolver2"); - private static final Set<String> STAX_HARDENER = set("StaxHardener", "StaxHardener$DtdSubsetFloor", "HardeningXMLInputFactory", HARDENING_EXCEPTION, - "FallbackDenyXMLResolver", "FallbackIgnoreXMLResolver"); + private static final Set<String> STAX_HARDENER = set("StaxHardener", "HardeningXMLInputFactory", "FallbackIgnoreXMLResolver"); /** - * TrAX, XPath and schema re-harden their sub-parsers through {@link SAXParserHardener#hardenSource}, so each builds on the full SAX closure below. + * TrAX, XPath and schema re-harden their sub-parsers through {@link SAXParserHardener#harden(Source)}, so each builds on the full SAX closure below. */ private static final Set<String> TRANSFORMER_HARDENER = saxParsersHardenerPlus("TransformerHardener", "HardeningTransformerFactory", - "HardeningTransformer", "HardeningTemplates", "FallbackDenyURIResolver", "SaxonProvider", "SaxonProvider$1", "SaxonProvider$HardenedConfiguration" + "HardeningTransformer", "HardeningTemplates", "FallbackIgnoreURIResolver", "SaxonProvider", "SaxonProvider$1", "SaxonProvider$HardenedConfiguration" , "SaxonProvider$SaxonProviderConfigurer"); private static final Set<String> XPATH_HARDENER = saxParsersHardenerPlus("XPathHardener", "SaxonProvider", "SaxonProvider$1", "SaxonProvider$HardenedConfiguration", "SaxonProvider$SaxonProviderConfigurer"); private static final Set<String> SCHEMA_HARDENER = saxParsersHardenerPlus("SchemaHardener", "HardeningSchemaFactory", "HardeningValidator", - "HardeningValidatorHandler", "HardeningSchema", "FallbackDenyLSResourceResolver"); + "HardeningValidatorHandler", "HardeningSchema", "FallbackIgnoreLSResourceResolver"); /** * Only the public {@link XmlFactories} entry, which news up every hardener, still pulls the whole library; this is its class count. */ - private static final int WHOLE_LIBRARY_SIZE = 33; + private static final int WHOLE_LIBRARY_SIZE = 30; /** * Entry points reported by the {@link #reportFootprint()} diagnostic, most-focused first, ending with the whole library. diff --git a/src/test/java/org/apache/commons/xml/TransformerDocumentTest.java b/src/test/java/org/apache/commons/xml/TransformerDocumentTest.java index 549f789..6f3dae8 100644 --- a/src/test/java/org/apache/commons/xml/TransformerDocumentTest.java +++ b/src/test/java/org/apache/commons/xml/TransformerDocumentTest.java @@ -29,6 +29,6 @@ class TransformerDocumentTest { @Test void hardenedTransformerBlocks() { - AttackTestSupport.assertTemplatesBlocks(AttackTestSupport.resourceSource("with-document.xsl")); + AttackTestSupport.assertTemplatesBlocksOrDoesNotLeak(AttackTestSupport.resourceSource("with-document.xsl")); } } diff --git a/src/test/java/org/apache/commons/xml/XIncludeTest.java b/src/test/java/org/apache/commons/xml/XIncludeTest.java index 9b17e06..1e3fbae 100644 --- a/src/test/java/org/apache/commons/xml/XIncludeTest.java +++ b/src/test/java/org/apache/commons/xml/XIncludeTest.java @@ -22,6 +22,7 @@ import static org.apache.commons.xml.AttackTestSupport.inputSource; import static org.apache.commons.xml.AttackTestSupport.resourceUrl; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -42,7 +43,7 @@ import org.xml.sax.helpers.DefaultHandler; /** - * Tests that XInclude resolution is denied by default on factories from {@link XmlFactories}, and that callers can + * Tests that XInclude resolution is blocked by default on factories from {@link XmlFactories}, and that callers can * allow-list specific resources via an {@link EntityResolver}. * * <p>Each case is exercised in both {@code parse="xml"} and {@code parse="text"} modes, and for both DOM and SAX @@ -64,7 +65,7 @@ class XIncludeTest { /** * Allow-lists the two fixture URLs, returning the appropriate in-memory content for each: {@link #RESOLVED_MARKER} * wrapped as XML for {@link #REFERENCED_XML}, and as plain text for {@link #REFERENCED_TEXT}. Anything else returns - * {@code null} so the hardening's deny-all floor refuses it. Mirrors a caller allow-listing trusted resources. + * {@code null} so the hardening's ignore-all floor empties it. Mirrors a caller allow-listing trusted resources. */ private static final class AllowListResolver implements EntityResolver { @@ -84,7 +85,7 @@ public InputSource resolveEntity(final String publicId, final String systemId) { } } - /** Resolver that resolves nothing, so the hardening's deny-all floor must refuse every lookup and never leak. */ + /** Resolver that resolves nothing, so the hardening's ignore-all floor must empty every lookup and never leak. */ private static final EntityResolver NO_OP_RESOLVER = (publicId, systemId) -> null; /** XML wrapper for xi:include in the given {@code parse} mode referencing {@code href}. */ @@ -195,7 +196,7 @@ public void characters(final char[] ch, final int start, final int length) { //endregion - //region Hardened factory: fails closed (throws) + //region Hardened factory: the include is never fetched @Test @Tag("dom") @@ -219,10 +220,10 @@ void hardenedDomBlocksParseText() throws Exception { final DocumentBuilderFactory factory = XmlFactories.newDocumentBuilderFactory(); factory.setNamespaceAware(true); assumeXIncludeAware(factory); - assertThrows(SAXException.class, () -> { - final DocumentBuilder builder = factory.newDocumentBuilder(); - builder.parse(input); - }, "Hardened DOM parse=text should throw"); + final Document doc = factory.newDocumentBuilder().parse(input); + final String text = doc.getDocumentElement().getTextContent(); + assertFalse(text.contains(LEAKED_MARKER), + "Hardened DOM parse=text must resolve the include to empty, not leak; got: " + text); } @Test @@ -247,10 +248,17 @@ void hardenedSaxBlocksParseText() throws Exception { final SAXParserFactory factory = XmlFactories.newSAXParserFactory(); factory.setNamespaceAware(true); assumeXIncludeAware(factory); - assertThrows(SAXException.class, () -> { - final XMLReader reader = factory.newSAXParser().getXMLReader(); - reader.parse(input); - }, "Hardened SAX parse=text should throw"); + final StringBuilder captured = new StringBuilder(); + final XMLReader reader = factory.newSAXParser().getXMLReader(); + reader.setContentHandler(new DefaultHandler() { + @Override + public void characters(final char[] ch, final int start, final int length) { + captured.append(ch, start, length); + } + }); + reader.parse(input); + assertFalse(captured.toString().contains(LEAKED_MARKER), + "Hardened SAX parse=text must resolve the include to empty, not leak; got: " + captured); } //endregion @@ -298,7 +306,7 @@ void hardenedDomNullResolverDoesNotLeak() throws Exception { final DocumentBuilder builder = factory.newDocumentBuilder(); builder.setEntityResolver(NO_OP_RESOLVER); assertThrows(SAXException.class, () -> builder.parse(input), - "a resolver that returns null must not leak: the deny-all floor blocks the real href"); + "a resolver that returns null must not leak: the ignore-all floor blocks the real href"); } @Test @@ -356,7 +364,7 @@ void hardenedSaxNullResolverDoesNotLeak() throws Exception { final XMLReader reader = factory.newSAXParser().getXMLReader(); reader.setEntityResolver(NO_OP_RESOLVER); assertThrows(SAXException.class, () -> reader.parse(input), - "a resolver that returns null must not leak: the deny-all floor blocks the real href"); + "a resolver that returns null must not leak: the ignore-all floor blocks the real href"); } //endregion @@ -386,8 +394,16 @@ void hardenReaderBlocksParseText() throws Exception { unhardenedFactory.setNamespaceAware(true); assumeXIncludeAware(unhardenedFactory); final XMLReader reader = XmlFactories.harden(unhardenedFactory.newSAXParser().getXMLReader()); - assertThrows(SAXException.class, () -> reader.parse(input), - "harden(reader) should block XInclude parse=text on reader with XInclude already enabled"); + final StringBuilder captured = new StringBuilder(); + reader.setContentHandler(new DefaultHandler() { + @Override + public void characters(final char[] ch, final int start, final int length) { + captured.append(ch, start, length); + } + }); + reader.parse(input); + assertFalse(captured.toString().contains(LEAKED_MARKER), + "harden(reader) parse=text must resolve the include to empty, not leak; got: " + captured); } @Test
