This is an automated email from the ASF dual-hosted git repository.
ppkarwasz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/commons-xml.git
The following commit(s) were added to refs/heads/main by this push:
new 9c84b10 fix: Restore hardened state after reset() (#36)
9c84b10 is described below
commit 9c84b10a3497940676db7b88533481679fc5d0a5
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Tue Aug 18 17:08:20 2026 +0200
fix: Restore hardened state after reset() (#36)
* fix: Restore hardened state after SAXParser.reset()
The JAXP reset contract reverts a parser to its just-created state, which
discards the reader-level configuration installed after creation, while the
wrapper kept serving its stale cached views. HardeningSAXParser.reset() now
drops the cached reader and parser views, so the next getXMLReader() or
getParser() call runs the freshly reset reader through the hardening funnel
again, matching HardeningDocumentBuilder.reset().
Assisted-By: Claude Fable 5 <[email protected]>
* fix: Restore hardened state after Validator.reset()
The JAXP reset contract reverts a validator to its just-created state,
which removes the resource-resolver floor installed by the constructor.
HardeningValidator.reset() now re-establishes the bare floor, matching
HardeningDocumentBuilder.reset().
Assisted-By: Claude Fable 5 <[email protected]>
* fix: Restore hardened state after Transformer.reset()
The JAXP reset contract reverts a transformer to its just-created state,
and several implementations clear the URI resolver installed by the
constructor. HardeningTransformer.reset() now re-establishes the floor,
seeded again with the factory's compile-time resolver, matching the other
hardened wrappers.
Assisted-By: Claude Fable 5 <[email protected]>
* test: Cover DocumentBuilder.reset()
HardeningDocumentBuilder.reset() already re-establishes its floor, but no
test exercised it; a regression there would go unnoticed.
Assisted-By: Claude Fable 5 <[email protected]>
---
.../org/apache/commons/xml/HardeningSAXParser.java | 14 ++-
.../apache/commons/xml/HardeningTransformer.java | 19 ++--
.../org/apache/commons/xml/HardeningValidator.java | 4 +-
.../org/apache/commons/xml/ResetHardeningTest.java | 118 +++++++++++++++++++++
4 files changed, 143 insertions(+), 12 deletions(-)
diff --git a/src/main/java/org/apache/commons/xml/HardeningSAXParser.java
b/src/main/java/org/apache/commons/xml/HardeningSAXParser.java
index d8bed29..5a79e74 100644
--- a/src/main/java/org/apache/commons/xml/HardeningSAXParser.java
+++ b/src/main/java/org/apache/commons/xml/HardeningSAXParser.java
@@ -68,6 +68,15 @@ public Parser getParser() throws SAXException {
return hardenedParser;
}
+ @Override
+ public void reset() {
+ delegate.reset();
+ // The JAXP reset contract reverts the delegate to its just-created
state, which strips the post-creation reader hardening.
+ // We reset the cached readers, so hardening can be applied again.
+ hardenedReader = null;
+ hardenedParser = null;
+ }
+
// <editor-fold defaultstate="collapsed" desc="Trivial delegation">
@Override
public Object getProperty(final String name) throws
SAXNotRecognizedException, SAXNotSupportedException {
@@ -94,11 +103,6 @@ public boolean isXIncludeAware() {
return delegate.isXIncludeAware();
}
- @Override
- public void reset() {
- delegate.reset();
- }
-
@Override
public void setProperty(final String name, final Object value) throws
SAXNotRecognizedException, SAXNotSupportedException {
delegate.setProperty(name, value);
diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformer.java
b/src/main/java/org/apache/commons/xml/HardeningTransformer.java
index a1b9322..6ab4f97 100644
--- a/src/main/java/org/apache/commons/xml/HardeningTransformer.java
+++ b/src/main/java/org/apache/commons/xml/HardeningTransformer.java
@@ -33,20 +33,32 @@
* 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>
+ * routes a caller's resolver through it rather than replacing it, so the
block cannot be dropped. {@link #reset()} re-establishes the floor, seeded
again with
+ * the factory's compile-time resolver, matching the just-constructed
state.</p>
*/
final class HardeningTransformer extends Transformer {
private final Transformer delegate;
+ /** Compile-time URIResolver snapshot the floor is seeded with, both at
construction and again on {@link #reset()}. */
+ private final URIResolver uriResolver;
+
private final FallbackIgnoreURIResolver floor;
HardeningTransformer(final Transformer delegate, final URIResolver
uriResolver) {
this.delegate = delegate;
+ this.uriResolver = uriResolver;
this.floor = new FallbackIgnoreURIResolver(uriResolver);
delegate.setURIResolver(floor);
}
+ @Override
+ public void reset() {
+ delegate.reset();
+ floor.setDelegate(uriResolver);
+ delegate.setURIResolver(floor);
+ }
+
@Override
public void setURIResolver(final URIResolver resolver) {
floor.setDelegate(resolver);
@@ -92,11 +104,6 @@ public Object getParameter(final String name) {
return delegate.getParameter(name);
}
- @Override
- public void reset() {
- delegate.reset();
- }
-
@Override
public void setErrorListener(final ErrorListener listener) {
delegate.setErrorListener(listener);
diff --git a/src/main/java/org/apache/commons/xml/HardeningValidator.java
b/src/main/java/org/apache/commons/xml/HardeningValidator.java
index cb30279..e67f1e0 100644
--- a/src/main/java/org/apache/commons/xml/HardeningValidator.java
+++ b/src/main/java/org/apache/commons/xml/HardeningValidator.java
@@ -33,7 +33,7 @@
/**
* {@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 an
ignore-all {@link LSResourceResolver} floor so {@code xsi:schemaLocation} is
not resolved at
- * validation time.
+ * validation time. {@link #reset()} re-establishes the bare ignore-all floor,
matching the just-constructed state.
*/
final class HardeningValidator extends Validator {
@@ -71,6 +71,8 @@ public LSResourceResolver getResourceResolver() {
@Override
public void reset() {
delegate.reset();
+ floor.setDelegate(null);
+ delegate.setResourceResolver(floor);
}
@Override
diff --git a/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
b/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
new file mode 100644
index 0000000..93c0de9
--- /dev/null
+++ b/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
@@ -0,0 +1,118 @@
+/*
+ * 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 static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.io.StringWriter;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.SAXParser;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.validation.Validator;
+
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.w3c.dom.Document;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+
+/**
+ * Checks that the JAXP {@code reset()} lifecycle methods do not strip the
hardening floors.
+ *
+ * <p>The JAXP reset contract returns an object to its just-created state, and
the stock JDK / Xerces implementations take that literally: they re-install
+ * their initial (null) resolvers, silently removing any floor the hardened
wrappers installed after creation. Each test resets a hardened object and
asserts
+ * that an external reference is still either blocked at parse or resolved to
empty content afterwards; the tests are skipped on platforms whose
+ * implementation does not support {@code reset()} at all (there the hardening
cannot be stripped in the first place).</p>
+ */
+class ResetHardeningTest {
+
+ /** systemId of the external general entity the floor must keep covering
after a reset (its content carries {@link AttackTestSupport#LEAKED_MARKER}). */
+ private static final String UNLISTED =
AttackTestSupport.resourceUrl("referenced.xml").toString();
+
+ private static String entityPayload(final String entitySystemId) {
+ return "<?xml version=\"1.0\"?>\n"
+ + "<!DOCTYPE root [\n <!ENTITY xxe SYSTEM \"" +
entitySystemId + "\">\n]>\n"
+ + "<root>&xxe;</root>";
+ }
+
+ @Test
+ @Tag("dom")
+ void documentBuilderResetKeepsEntityResolverFloor() throws Exception {
+
Assumptions.assumeTrue(AttackTestSupport.DOM_RESOLVES_INTERNAL_ENTITIES,
"platform DOM does not resolve user-defined entities");
+ final DocumentBuilder builder =
XmlFactories.newDocumentBuilderFactory().newDocumentBuilder();
+ AttackTestSupport.assumeDoesNotThrow(builder::reset);
+ try {
+ final Document doc =
builder.parse(AttackTestSupport.inputSource(entityPayload(UNLISTED)));
+
assertFalse(doc.getDocumentElement().getTextContent().contains(AttackTestSupport.LEAKED_MARKER),
"external entity leaked after reset");
+ } catch (final SAXException blocked) {
+ // Acceptable: rejected at parse rather than resolved to empty.
+ }
+ }
+
+ @Test
+ @Tag("sax")
+ void saxParserResetKeepsEntityResolverFloor() throws Exception {
+ final SAXParser parser =
XmlFactories.newSAXParserFactory().newSAXParser();
+ // Materialize the hardened reader before the reset, so a stale cached
wrapper would be observable.
+ parser.getXMLReader();
+ AttackTestSupport.assumeDoesNotThrow(parser::reset);
+ final XMLReader reader = parser.getXMLReader();
+ final String text;
+ try {
+ text = AttackTestSupport.captureCharacters(reader,
entityPayload(UNLISTED));
+ } catch (final SAXException blocked) {
+ return; // Acceptable: rejected at parse rather than resolved to
empty.
+ }
+ assertFalse(text.contains(AttackTestSupport.LEAKED_MARKER), "external
entity leaked after reset:\n" + text);
+ }
+
+ @Test
+ @Tag("schema")
+ void validatorResetKeepsResourceResolverFloor() throws Exception {
+ // A Schema built without sources validates against the instance's
xsi:schemaLocation hints, so the resolver floor is the only barrier between the
+ // validator and the external schema fetch.
+ final Validator validator =
XmlFactories.newSchemaFactory(XMLConstants.W3C_XML_SCHEMA_NS_URI).newSchema().newValidator();
+ AttackTestSupport.assumeDoesNotThrow(validator::reset);
+ validator.setErrorHandler(AttackTestSupport.STRICT_REPORTER);
+ // schema-location-instance.xml hints at schema-location.xsd, which
declares its root: a validator whose floor was stripped fetches it and validates
+ // cleanly, while the floor resolves the hint to empty content, which
fails the validation.
+ AttackTestSupport.assertParseFails(() ->
validator.validate(AttackTestSupport.resourceSource("schema-location-instance.xml")),
+ "Validator after reset", SAXException.class,
SecurityException.class);
+ }
+
+ @Test
+ @Tag("trax")
+ void transformerResetKeepsUriResolverFloor() throws Exception {
+ // with-document.xsl copies document('referenced.xml') into the output
at transform time, so a transformer whose floor was stripped leaks the marker.
+ final Transformer transformer = XmlFactories.newTransformerFactory()
+
.newTemplates(AttackTestSupport.resourceSource("with-document.xsl")).newTransformer();
+ AttackTestSupport.assumeDoesNotThrow(transformer::reset);
+ final StringWriter sink = new StringWriter();
+ try {
+ transformer.transform(AttackTestSupport.streamSource("<root/>"),
new StreamResult(sink));
+ } catch (final TransformerException blocked) {
+ return; // Acceptable: rejected at transform rather than resolved
to empty.
+ }
+ assertFalse(sink.toString().contains(AttackTestSupport.LEAKED_MARKER),
"document() leaked after reset:\n" + sink);
+ }
+}
