This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/commons-secure-xml.git
The following commit(s) were added to refs/heads/main by this push:
new fa90fe8 Resolve each unresolved URI to a fresh empty document (#69)
fa90fe8 is described below
commit fa90fe81ddc9db5d6315382d199450a0c9c7d23e
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Tue Sep 1 13:27:53 2026 +0200
Resolve each unresolved URI to a fresh empty document (#69)
The URIResolver floor answered every unresolved reference with a
DOMSource over one shared static Document; the Source escapes to the
consumer, so a component mutating a document it received would surface
its changes in every later resolution, process-wide. Follow the JDK's
XSLTC pattern and build a fresh empty document per resolution, using
the same secured, namespace-aware factory selection as the Source
rewrite (an upgrade over the previous raw JAXP lookup, and it now
honors overrideDefaultParser). Creation failure is wrapped in
IllegalStateException per call instead of ExceptionInInitializerError.
The threat model now states the matching scope rule: modifying a
mutable object a JAXP method returned or stored (the capability behind
the SpotBugs expose-internal-representation patterns) presumes an
adversary already running in the process, which the model does not
defend against — the isolation here is robustness, not a defended
boundary.
Assisted-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CLnTBsvmYtxzNTWVGNyz33
---
.../xml/secure/FallbackIgnoreURIResolver.java | 38 +++++++++-------------
src/site/markdown/threat_model.md | 17 +++++++++-
.../xml/secure/FallbackIgnoreURIResolverTest.java | 18 ++++++++--
3 files changed, 48 insertions(+), 25 deletions(-)
diff --git
a/src/main/java/org/apache/commons/xml/secure/FallbackIgnoreURIResolver.java
b/src/main/java/org/apache/commons/xml/secure/FallbackIgnoreURIResolver.java
index 4fc0e00..ccda148 100644
--- a/src/main/java/org/apache/commons/xml/secure/FallbackIgnoreURIResolver.java
+++ b/src/main/java/org/apache/commons/xml/secure/FallbackIgnoreURIResolver.java
@@ -39,8 +39,8 @@
* non-{@code null} {@link Source}; anything left unresolved resolves to an
empty {@link Source}, so the external resource is neither fetched nor leaked.
* </p>
* <p>
- * The shape of that empty {@link Source} is supplied by the caller: the
default is a well-formed empty DOM document (which every stock TrAX consumer
accepts),
- * while the Saxon path supplies {@code EmptySource.getInstance()} so its
consumers get the "empty" shape they expect.
+ * The shape of that empty {@link Source} is supplied by the caller: the
default is a fresh, well-formed empty DOM document per resolution (which every
stock
+ * TrAX consumer accepts), while the Saxon path supplies {@code
EmptySource.getInstance()} so its consumers get the "empty" shape they expect.
* </p>
* <p>
* An opted-in {@link javax.xml.transform.stream.StreamSource} or reader-less
{@link javax.xml.transform.sax.SAXSource} is rewritten to carry a secure reader
@@ -51,39 +51,32 @@
final class FallbackIgnoreURIResolver implements URIResolver {
/**
- * Backing for the default ignore outcome. Consumers parse the resolved
{@link Source}, and an empty character stream is not a well-formed XML document
- * (XSLTC rejects it for {@code document()} and for an ignored {@code
xsl:include}/{@code xsl:import}), so the default supplier answers with a
well-formed
- * empty document that evaluates to no content. It is never mutated, so
one instance serves every resolution.
+ * Creates the empty document backing the default ignore outcome.
*
- * @see #newEmptyDocument()
- */
- private static final Document EMPTY_DOCUMENT;
-
- static {
- EMPTY_DOCUMENT =
newEmptyDocument(DocumentBuilderFactory.newInstance());
- }
-
- /**
- * Creates a new empty document.
+ * <p>Consumers parse the resolved {@link Source},
+ * and an empty character stream is not a well-formed XML document
+ * (XSLTC rejects it for {@code document()} and for an ignored {@code
xsl:include}/{@code xsl:import}),
+ * so the default supplier answers with a well-formed empty document that
evaluates to no content.</p>
+ *
+ * <p>The document escapes to the consumer with the resolved {@link
Source}, so each resolution gets its own: whatever a consumer does to a
document it
+ * received cannot surface in another resolution.</p>
*
- * @param factory the factory to use to create a new document builder.
+ * @param factory the factory to create the document builder with.
* @return a new empty document.
- * @throws SecureException Thrown if a {@link DocumentBuilder}
cannot be created which satisfies the configuration requested.
- * @throws ExceptionInInitializerError Thrown from a factory in case of a
{@link java.util.ServiceConfigurationError service
- * configuration error} or if the
implementation is not available or cannot be instantiated.
+ * @throws IllegalStateException Thrown if the factory cannot supply a
{@link javax.xml.parsers.DocumentBuilder} satisfying its configuration.
*/
private static Document newEmptyDocument(final DocumentBuilderFactory
factory) {
try {
return factory.newDocumentBuilder().newDocument();
} catch (final ParserConfigurationException e) {
- throw new ExceptionInInitializerError(e);
+ throw new IllegalStateException(e);
}
}
private URIResolver delegate;
/**
- * Produces the empty {@link Source} returned for an unresolved reference;
a new value per call keeps callers from mutating a shared Source.
+ * Produces the empty {@link Source} returned for an unresolved reference.
*/
private final Supplier<Source> emptySource;
@@ -102,7 +95,8 @@ private static Document newEmptyDocument(final
DocumentBuilderFactory factory) {
*/
FallbackIgnoreURIResolver(final URIResolver delegate, final
Supplier<Source> emptySource, final BooleanSupplier overrideDefaultParser) {
this.delegate = delegate;
- this.emptySource = emptySource != null ? emptySource : () -> new
DOMSource(EMPTY_DOCUMENT);
+ this.emptySource = emptySource != null ? emptySource
+ : () -> new
DOMSource(newEmptyDocument(SecureDocumentBuilderFactory.newNSInstance(overrideDefaultParser.getAsBoolean())));
this.overrideDefaultParser = overrideDefaultParser;
}
diff --git a/src/site/markdown/threat_model.md
b/src/site/markdown/threat_model.md
index f63585a..3307514 100644
--- a/src/site/markdown/threat_model.md
+++ b/src/site/markdown/threat_model.md
@@ -73,6 +73,14 @@ The library secures what it creates;
it does not re-harden what you built,
because your reader's settings are indistinguishable from configuration you
chose deliberately.
+It also holds for the objects a produced instance returns:
+exploiting the exposure the SpotBugs patterns
+[*May expose internal representation by incorporating reference to mutable
object*](https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#ei2-may-expose-internal-representation-by-incorporating-reference-to-mutable-object-ei-expose-rep2)
+and [*… by returning reference to mutable
object*](https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#ei-may-expose-internal-representation-by-returning-reference-to-mutable-object-ei-expose-rep)
+warn about presumes an adversary already running in the process,
+a capability the adversary of this model does not have.
+Modifying a shared mutable object is a bug, not a vulnerability.
+
### What is in Scope
- The securing recipes applied by `org.apache.commons.xml.secure`.
@@ -255,6 +263,13 @@ and reports against a factory reconfigured in any of the
ways below are out of s
- **Caller-supplied top-level URIs.** A URI passed directly to a parse call
(`DocumentBuilder.parse(String)`,
`StreamSource(systemId)`, a `SAXSource` built from a system id) is fetched
as-is by the JAXP implementation without
consulting the securing layer. Restrict it yourself if the URI is untrusted.
+- **Mutating returned objects.**
+ Modifying an object a produced instance returned —
+ the `Document` of a parse or of an empty resolution,
+ a `Source` handed back by a resolver or by `getAssociatedStylesheet` —
+ is same-process capability, like reconfiguring the factory
+ (see [Adversary model and trust
boundary](#adversary-model-and-trust-boundary)).
+ A report premised on a same-process component mutating a JAXP method's
result is out of scope.
- **Caller-supplied parser instances.**
A parser built outside `org.apache.commons.xml.secure` and handed to a
produced instance is used as configured:
a `SAXSource` carrying its own `XMLReader`,
@@ -327,7 +342,7 @@ A report judged against this model receives exactly one of:
| Disposition | Meaning |
| --- | --- |
| `VALID` | A factory or instance used as delivered fails to provide a
guarantee its Javadoc states (for example, a secured parser still resolves an
external entity, or a documented processing limit is not applied). |
-| `OUT-OF-SCOPE: reconfigured` | A reserved setting was loosened, or a
resolver was installed, on the factory or a produced instance before the
reported behavior (see [What is out of scope](#what-is-out-of-scope)). |
+| `OUT-OF-SCOPE: reconfigured` | A reserved setting was loosened, a resolver
was installed, or a returned object was mutated, on the factory or a produced
instance before the reported behavior (see [What is out of
scope](#what-is-out-of-scope)). |
| `OUT-OF-SCOPE: caller input` | The behavior follows from a top-level URI, a
parser instance the caller constructed outside the library, or other input the
caller passed directly to a parse call. |
| `OUT-OF-SCOPE: foreign implementation` | The behavior is in a JAXP
implementation that does not respect the contract of the settings a securing
recipe requires, or is a defect in the underlying JAXP implementation itself. |
| `OUT-OF-SCOPE: unsupported runtime` | The behavior is demonstrated only on a
runtime the guarantees are not defined on, such as Android on any API level
(see **Supported runtimes** under [Assumptions about the
environment](#assumptions-about-the-environment)). |
diff --git
a/src/test/java/org/apache/commons/xml/secure/FallbackIgnoreURIResolverTest.java
b/src/test/java/org/apache/commons/xml/secure/FallbackIgnoreURIResolverTest.java
index b992eef..4f75bc8 100644
---
a/src/test/java/org/apache/commons/xml/secure/FallbackIgnoreURIResolverTest.java
+++
b/src/test/java/org/apache/commons/xml/secure/FallbackIgnoreURIResolverTest.java
@@ -17,6 +17,8 @@
package org.apache.commons.xml.secure;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
@@ -33,6 +35,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledInNativeImage;
+import org.w3c.dom.Document;
class FallbackIgnoreURIResolverTest {
@@ -40,14 +43,14 @@ class FallbackIgnoreURIResolverTest {
// Mockito generates the mock class and its plugin proxies at run time —
impossible in a closed-world native image,
// so the stubbed factory this error path needs cannot be built there.
@DisabledInNativeImage
- void newEmptyDocumentThrowsExceptionInInitializerError() throws Exception {
+ void newEmptyDocumentWrapsConfigurationFailure() throws Exception {
final DocumentBuilderFactory factory =
mock(DocumentBuilderFactory.class);
final ParserConfigurationException failure = new
ParserConfigurationException("test");
when(factory.newDocumentBuilder()).thenThrow(failure);
final Method method =
FallbackIgnoreURIResolver.class.getDeclaredMethod("newEmptyDocument",
DocumentBuilderFactory.class);
method.setAccessible(true);
final InvocationTargetException exception =
assertThrows(InvocationTargetException.class, () -> method.invoke(null,
factory));
- final ExceptionInInitializerError error =
(ExceptionInInitializerError) exception.getCause();
+ final IllegalStateException error = (IllegalStateException)
exception.getCause();
assertSame(failure, error.getCause());
}
@@ -69,4 +72,15 @@ void resolvesDelegatedAndFallbackSources() throws Exception {
System.clearProperty(SecureException.THROW_ON_UNRESOLVED);
}
}
+
+ @Test
+ void resolvesFreshEmptyDocumentPerResolution() throws Exception {
+ final FallbackIgnoreURIResolver resolver = new
FallbackIgnoreURIResolver(null, null, () -> false);
+ final Document first = (Document) ((DOMSource)
resolver.resolve("href", "base")).getNode();
+ assertNull(first.getDocumentElement());
+ first.appendChild(first.createElement("planted"));
+ final Document second = (Document) ((DOMSource)
resolver.resolve("href", "base")).getNode();
+ assertNotSame(first, second);
+ assertNull(second.getDocumentElement());
+ }
}
