Factory classes live in the {@code org.apache.harmony.xml.parsers.*} package, but DOM and SAX are backed by two different engines:
- *What the SAX/Expat surface exposes:
- *What the DOM/KXmlParser surface exposes:
- *The SAX path installs a {@link DtdAwareDenyResolver} as both {@link EntityResolver} and {@link LexicalHandler}: it allows the external subset to load - * silently (so a DOCTYPE that names an external DTD but does not use it parses) and throws on every external general or parameter entity reference.
- */ -final class AndroidProvider { - - /** - * Resolver that denies every external resource lookup, except the external DTD subset declared by the DOCTYPE - * - *Merely declaring an external subset does not cause the parse to throw.
- */ - private static final class DtdAwareDenyResolver extends DefaultHandler2 { - - private static String forbiddenMessage(final String publicId, final String systemId) { - return String.format("External Entity: failed to read external entity (publicId='%s', systemId='%s'); external entity access is denied.", - publicId, systemId); - } - - private String dtdPublicId; - private String dtdSystemId; - private boolean inDtd; - - @Override - public void endDTD() { - inDtd = false; - } - - @Override - public InputSource resolveEntity(final String publicId, final String systemId) throws SAXException { - if (inDtd && Objects.equals(publicId, dtdPublicId) && Objects.equals(systemId, dtdSystemId)) { - return null; - } - throw new SAXException(forbiddenMessage(publicId, systemId)); - } - - @Override - public void startDTD(final String name, final String publicId, final String systemId) { - inDtd = true; - dtdPublicId = publicId; - dtdSystemId = systemId; - } - } - - /** - * {@link SAXParser} wrapper whose {@link #getXMLReader()} returns a {@link GuardedXMLReader}. - */ - private static final class GuardedSAXParser extends DelegatingSAXParser { - - private final XMLReader guardedReader; - - GuardedSAXParser(final SAXParser delegate, final XMLReader guardedReader) { - super(delegate); - this.guardedReader = guardedReader; - } - - @Override - public XMLReader getXMLReader() { - return guardedReader; - } - } - - /** - * {@link SAXParserFactory} wrapper that produces {@link GuardedSAXParser}s. - */ - private static final class GuardedSAXParserFactory extends DelegatingSAXParserFactory { - - GuardedSAXParserFactory(final SAXParserFactory delegate) { - super(delegate); - } - - @Override - public SAXParser newSAXParser() throws ParserConfigurationException, SAXException { - final SAXParser parser = super.newSAXParser(); - return new GuardedSAXParser(parser, configure(parser.getXMLReader())); - } - } - - /** - * {@link XMLReader} wrapper that surfaces ExpatReader's conflicting-feature error at {@code setFeature} time rather than at {@code parse} time. - * - *Android's {@code ExpatReader.parse()} throws {@link SAXNotSupportedException} when {@code namespaces} and {@code namespace-prefixes} are both - * enabled. Reporting the error at configuration time lets consumers, such as Apache Xalan's identity transformer, catch the exception and still parse - * the document. Without the wrapper, parsing fails.
- */ - static final class GuardedXMLReader extends DelegatingXMLReader { - - GuardedXMLReader(final XMLReader delegate) { - super(delegate); - } - - @Override - public void setFeature(final String name, final boolean value) throws SAXNotRecognizedException, SAXNotSupportedException { - if (value - && (NAMESPACE_PREFIXES_FEATURE.equals(name) && super.getFeature(NAMESPACES_FEATURE) - || NAMESPACES_FEATURE.equals(name) && super.getFeature(NAMESPACE_PREFIXES_FEATURE))) { - throw new SAXNotSupportedException("ExpatReader cannot have both '" + NAMESPACES_FEATURE + "' and '" + NAMESPACE_PREFIXES_FEATURE + "' " + - "enabled simultaneously"); - } - super.setFeature(name, value); - } - } - - private static final String LEXICAL_HANDLER_PROPERTY = "http://xml.org/sax/properties/lexical-handler"; - - private static final String NAMESPACES_FEATURE = "http://xml.org/sax/features/namespaces"; - - private static final String NAMESPACE_PREFIXES_FEATURE = "http://xml.org/sax/features/namespace-prefixes"; - - static SAXParserFactory configure(final SAXParserFactory factory) { - return new GuardedSAXParserFactory(factory); - } - - static XMLReader configure(final XMLReader reader) { - if (reader instanceof GuardedXMLReader) { - return reader; - } - final DtdAwareDenyResolver resolver = new DtdAwareDenyResolver(); - reader.setEntityResolver(resolver); - try { - reader.setProperty(LEXICAL_HANDLER_PROPERTY, resolver); - } catch (final SAXException ignore) { - // ExpatReader recognizes the lexical-handler property; if a future replacement does not, fall through and lose subset-vs-entity discrimination. - } - return new GuardedXMLReader(reader); - } - - private AndroidProvider() { - } -} \ No newline at end of file diff --git a/src/main/java/org/apache/commons/xml/HardeningSAXParser.java b/src/main/java/org/apache/commons/xml/HardeningSAXParser.java new file mode 100644 index 00000000..766c399f --- /dev/null +++ b/src/main/java/org/apache/commons/xml/HardeningSAXParser.java @@ -0,0 +1,65 @@ +/* + * 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.parsers.SAXParser; + +import org.xml.sax.Parser; +import org.xml.sax.SAXException; +import org.xml.sax.XMLReader; +import org.xml.sax.helpers.XMLReaderAdapter; + +/** + * {@link SAXParser} that exposes a hardened {@link XMLReader} and a matching SAX 1 {@link Parser}. + * + *Both views are produced from the same hardened reader, so a caller reaching the parser through either the SAX 2 ({@link #getXMLReader()}) or the legacy + * SAX 1 ({@link #getParser()}) path gets the same hardening. The SAX 1 view matters because some consumers, such as Xalan's identity transformer, still ask + * for a {@link Parser}.
+ * + *The hardened reader is computed lazily on first access and cached: hardening an {@link XMLReader} can install a fresh wrapper (Android's Expat path), so + * every parse must run through the same instance. The {@code parse(...)} overloads inherited from {@link SAXParser} dispatch virtually to {@link #getXMLReader()} + * and {@link #getParser()}, so they too run through the hardened views without further overrides.
+ */ +final class HardeningSAXParser extends DelegatingSAXParser { + + private XMLReader hardenedReader; + private Parser hardenedParser; + + HardeningSAXParser(final SAXParser delegate) { + super(delegate); + } + + @Override + public XMLReader getXMLReader() throws SAXException { + if (hardenedReader == null) { + hardenedReader = SAXParserHardener.hardenReader(super.getXMLReader()); + } + return hardenedReader; + } + + @Override + @SuppressWarnings("deprecation") + public Parser getParser() throws SAXException { + if (hardenedParser == null) { + final XMLReader reader = getXMLReader(); + // Reuse the reader directly if it already is a SAX 1 parser; otherwise adapt it, so the SAX 1 path runs through the same hardened reader. + hardenedParser = reader instanceof Parser ? (Parser) reader : new XMLReaderAdapter(reader); + } + return hardenedParser; + } +} diff --git a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java index a096c049..df78abef 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java @@ -17,8 +17,6 @@ package org.apache.commons.xml; -import java.util.function.UnaryOperator; - import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -27,24 +25,20 @@ import org.xml.sax.XMLReader; /** - * Universal SAX wrapper that defers per-parser hardening to a supplied {@link XMLReader} hardener. + * Universal SAX factory wrapper that funnels every produced parser through {@link SAXParserHardener#hardenReader(XMLReader)}. * - *{@link SAXParserFactory} has no property API, only a feature API. Therefore, complex configuration must be performed on each new - * {@link XMLReader} parser.
+ *{@link SAXParserFactory} exposes only a feature API and no property API, so the per-parse hardening (limits, entity blocking, implementation-specific + * fixups) has to run on each {@link XMLReader} the factory produces. This wrapper returns a {@link HardeningSAXParser}, which applies that hardening lazily to + * both the SAX 2 {@link XMLReader} and the SAX 1 {@link org.xml.sax.Parser} it exposes.
*/ final class HardeningSAXParserFactory extends DelegatingSAXParserFactory { - private final UnaryOperatorExternal Xerces carries its limits on an {@code org.apache.xerces.util.SecurityManager} instance reachable through the + * {@value XercesProvider#XERCES_SECURITY_MANAGER_PROPERTY} property. The stock JDK reader is itself a Xerces fork that exposes the same property, so a probe + * cannot tell the two apart; the external distribution is therefore matched by class name, and every other reader (the stock JDK, a Saxon-picked JDK reader, + * any future attribute-based parser) takes the JDK limit properties. Neither path throws if the reader declines a limit.
* * @param reader The target reader to modify. */ - static void applyToJdkXmlReader(final XMLReader reader) { - JDK_LIMITS.forEach((name, supplier) -> setProperty(reader, name, Integer.toString(supplier.getAsInt()))); + static void tryApply(final XMLReader reader) { + if (EXTERNAL_XERCES_SAX_READER.equals(reader.getClass().getName())) { + try { + // External Xerces: tighten the SecurityManager it already installed under FSP to JDK 25 limits. + applyToXerces(reader.getProperty(XercesProvider.XERCES_SECURITY_MANAGER_PROPERTY)); + } catch (final SAXNotRecognizedException | SAXNotSupportedException e) { + throw new HardeningException("Failed to read Xerces security manager from XMLReader", e); + } + return; + } + // Pin the JDK limit properties to JDK 25 secure values; skip silently any property the reader does not recognize. + JDK_LIMITS.forEach((name, supplier) -> trySetProperty(reader, name, Integer.toString(supplier.getAsInt()))); } /** diff --git a/src/main/java/org/apache/commons/xml/Resolvers.java b/src/main/java/org/apache/commons/xml/Resolvers.java index 33867df9..59772cea 100644 --- a/src/main/java/org/apache/commons/xml/Resolvers.java +++ b/src/main/java/org/apache/commons/xml/Resolvers.java @@ -113,8 +113,7 @@ private DenyAll() { /** * Returns an empty input for every external resource lookup so the parse can continue without replacement content. * - *Only an {@link XMLResolver} flavour is exposed: schema and XSLT compile paths must always deny imports, and SAX/DOM use the deny-all hooks plus - * {@link AndroidProvider}'s subset-aware resolver where needed.
+ *Only an {@link XMLResolver} flavour is exposed: schema and XSLT compile paths must always deny imports, and SAX/DOM use the deny-all hooks.
*/ static final class IgnoreAll { diff --git a/src/main/java/org/apache/commons/xml/SAXParserHardener.java b/src/main/java/org/apache/commons/xml/SAXParserHardener.java new file mode 100644 index 00000000..9e99ed50 --- /dev/null +++ b/src/main/java/org/apache/commons/xml/SAXParserHardener.java @@ -0,0 +1,188 @@ +/* + * 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.apache.commons.xml.JaxpSetters.setFeature; +import static org.apache.commons.xml.JaxpSetters.setOptionalFeature; +import static org.apache.commons.xml.JaxpSetters.trySetProperty; + +import java.util.Objects; + +import javax.xml.XMLConstants; +import javax.xml.parsers.SAXParserFactory; + +import org.xml.sax.EntityResolver; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; +import org.xml.sax.XMLReader; +import org.xml.sax.ext.DefaultHandler2; +import org.xml.sax.ext.LexicalHandler; + +/** + * Capability-driven hardening for any {@link SAXParserFactory} on the classpath. + * + *Rather than branching on the implementation class, {@link #harden(SAXParserFactory)} probes what the parser supports and adapts. Because + * {@link SAXParserFactory} exposes only a feature API and no property API, the per-parse configuration runs on each {@link XMLReader} the factory produces, + * funnelled through {@link HardeningSAXParserFactory} into {@link #hardenReader(XMLReader)}:
+ *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 names an unused external + * subset. Tracking the subset's identifiers through the {@link LexicalHandler} DTD events lets this resolver allow exactly that one lookup and throw on + * everything else. It is stateful, so a fresh instance is installed per reader.
+ */ + private static final class DtdAwareDenyResolver extends DefaultHandler2 { + + private static String forbiddenMessage(final String publicId, final String systemId) { + return String.format("External Entity: failed to read external entity (publicId='%s', systemId='%s'); external entity access is denied.", + publicId, systemId); + } + + private String dtdPublicId; + private String dtdSystemId; + private boolean inDtd; + + @Override + public void endDTD() { + inDtd = false; + } + + @Override + public InputSource resolveEntity(final String publicId, final String systemId) throws SAXException { + if (inDtd && Objects.equals(publicId, dtdPublicId) && Objects.equals(systemId, dtdSystemId)) { + return null; + } + throw new SAXException(forbiddenMessage(publicId, systemId)); + } + + @Override + public void startDTD(final String name, final String publicId, final String systemId) { + inDtd = true; + dtdPublicId = publicId; + dtdSystemId = systemId; + } + } + + /** + * Wrapper around Android's {@code org.apache.harmony.xml.ExpatReader} that surfaces its {@code namespace-prefixes} limitation at configuration time. + * + *ExpatReader does not actually support the {@code namespace-prefixes} feature: enabling it is accepted by {@code setFeature} but fails later, during + * {@code parse}, with a {@link SAXNotSupportedException}. Reporting the rejection eagerly from {@link #setFeature(String, boolean)} lets consumers that probe + * the feature, such as Xalan's identity transformer, catch the exception and fall back instead of failing the whole parse.
+ */ + static final class ExpatReaderWrapper extends DelegatingXMLReader { + + private static final String NAMESPACE_PREFIXES_FEATURE = "http://xml.org/sax/features/namespace-prefixes"; + + ExpatReaderWrapper(final XMLReader delegate) { + super(delegate); + } + + @Override + public void setFeature(final String name, final boolean value) throws SAXNotRecognizedException, SAXNotSupportedException { + if (value && NAMESPACE_PREFIXES_FEATURE.equals(name)) { + throw new SAXNotSupportedException("ExpatReader does not support enabling the '" + NAMESPACE_PREFIXES_FEATURE + "' feature"); + } + super.setFeature(name, value); + } + } + + /** Class name of Android's Harmony-based {@link SAXParserFactory}, backed by the native Expat parser. */ + private static final String ANDROID_SAX_PARSER_FACTORY = "org.apache.harmony.xml.parsers.SAXParserFactoryImpl"; + + /** 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"; + + static SAXParserFactory harden(final SAXParserFactory factory) { + // Required: enables the implementation's security manager, which carries the limits. Android's Expat rejects FSP, so it is skipped there. + if (!ANDROID_SAX_PARSER_FACTORY.equals(factory.getClass().getName())) { + setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); + } + // The per-parse hardening (limits, entity blocking, Android fixups) lives in hardenReader() because SAXParserFactory has no property API. + return new HardeningSAXParserFactory(factory); + } + + /** + * Hardens an existing {@link XMLReader}. + * + * @param reader the reader to harden; never {@code null}. + * @return a hardened reader. + * @throws IllegalStateException if a required hardening setting cannot be applied to the underlying implementation. + */ + static XMLReader hardenReader(final XMLReader reader) { + if (reader instanceof ExpatReaderWrapper) { + // Already hardened (e.g. handed back through XmlFactories.harden(XMLReader)); applying the Expat fixups again would be redundant. + return reader; + } + if (ANDROID_EXPAT_READER.equals(reader.getClass().getName())) { + // Expat ignores external fetches when no resolver is set; install one that fails on external entities but lets an unused external subset load. + final DtdAwareDenyResolver resolver = new DtdAwareDenyResolver(); + reader.setEntityResolver(resolver); + // The resolver needs the DTD-boundary events to tell the subset apart from entities; Expat recognizes the lexical-handler property. + trySetProperty(reader, LEXICAL_HANDLER_PROPERTY, resolver); + // Reject the unsupported namespace-prefixes feature eagerly rather than mid-parse. + return new ExpatReaderWrapper(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); + // Optional, implementation-based: JDK limit properties or Xerces' SecurityManager. + Limits.tryApply(reader); + // ACCESS_EXTERNAL_* support is the dividing capability between JAXP 1.5 implementations and older ones. + if (trySetProperty(reader, XMLConstants.ACCESS_EXTERNAL_DTD, "") + && trySetProperty(reader, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "")) { + // Honoured (stock JDK): the JAXP 1.5 properties block external fetches, so the bare reader is already hardened. + return reader; + } + // Rejected: external Xerces ignores ACCESS_EXTERNAL_*; install a deny-all resolver, the only block. + reader.setEntityResolver(Resolvers.DenyAll.ENTITY2); + return reader; + } + + private SAXParserHardener() { + } +} diff --git a/src/main/java/org/apache/commons/xml/StockJdkProvider.java b/src/main/java/org/apache/commons/xml/StockJdkProvider.java index 99a7d8d2..751c60fe 100644 --- a/src/main/java/org/apache/commons/xml/StockJdkProvider.java +++ b/src/main/java/org/apache/commons/xml/StockJdkProvider.java @@ -51,8 +51,7 @@ * when the subset that would have declared the entity was skipped, so no extra hook is needed. * * - *SAX hardening lives in {@link #configure(XMLReader)}: {@link SAXParserFactory} has no property API, so the {@link HardeningSAXParserFactory} wrapper - * funnels each produced parser's {@link XMLReader} through that method.
+ *SAX hardening is not handled here: it is capability-driven across all implementations and lives in {@link SAXParserHardener}.
*/ final class StockJdkProvider { @@ -62,37 +61,10 @@ final class StockJdkProvider { private static final String FEATURE_OVERRIDE_DEFAULT_PARSER = "jdk.xml.overrideDefaultParser"; /** - * 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"; - - /** - * Zephyr property: skip external DTD subset loading entirely (StAX equivalent of {@link #XERCES_LOAD_EXTERNAL_DTD} {@code = false}). + * Zephyr property: skip external DTD subset loading entirely (StAX equivalent of the Xerces {@code load-external-dtd} feature {@code = false}). */ private static final String ZEPHYR_IGNORE_EXTERNAL_DTD = "http://java.sun.com/xml/stream/properties/ignore-external-dtd"; - static SAXParserFactory configure(final SAXParserFactory factory) { - // Required: enables the JDK XMLSecurityManager limits. - setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); - // Useful: namespaces should be recognized by default - factory.setNamespaceAware(true); - // The remaining hardening (limits, ACCESS_EXTERNAL_*) lives in the XMLReader configure() because SAXParserFactory has no property API. - return new HardeningSAXParserFactory(factory, StockJdkProvider::configure); - } - - static XMLReader configure(final XMLReader reader) { - // Required: enables the JDK XMLSecurityManager limits on a raw reader (e.g. one Saxon picked). - setFeature(reader, XMLConstants.FEATURE_SECURE_PROCESSING, true); - // Let DOCTYPE-only documents parse silently without SSRF: skip the external DTD subset on non-validating parsers. - setFeature(reader, XERCES_LOAD_EXTERNAL_DTD, false); - // Defense-in-depth: pin to JDK 25 limits so older JDKs do not fall back to looser secure values. - Limits.applyToJdkXmlReader(reader); - // Defense-in-depth: already FSP-secure defaults, set explicitly so they are not relaxed via system property. - setProperty(reader, XMLConstants.ACCESS_EXTERNAL_DTD, ""); - setProperty(reader, XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); - return reader; - } - static XMLInputFactory configure(final XMLInputFactory factory) { // Required: XMLInputFactory rejects FSP, so the limits below are the only way to enable JDK XMLSecurityManager caps on the StAX path. Limits.applyToJdkStax(factory); diff --git a/src/main/java/org/apache/commons/xml/XercesProvider.java b/src/main/java/org/apache/commons/xml/XercesProvider.java index 4634c9f9..f5ad9c36 100644 --- a/src/main/java/org/apache/commons/xml/XercesProvider.java +++ b/src/main/java/org/apache/commons/xml/XercesProvider.java @@ -20,8 +20,6 @@ import static org.apache.commons.xml.JaxpSetters.setFeature; import javax.xml.XMLConstants; -import javax.xml.parsers.SAXParser; -import javax.xml.parsers.SAXParserFactory; import javax.xml.validation.Schema; import javax.xml.validation.SchemaFactory; import javax.xml.validation.Validator; @@ -29,13 +27,13 @@ import org.xml.sax.SAXNotRecognizedException; import org.xml.sax.SAXNotSupportedException; -import org.xml.sax.XMLReader; /** * Hardening recipes for the external Apache Xerces distribution (the {@code xerces:xercesImpl} artifact). * *Factory classes live in the {@code org.apache.xerces.*} package. External Xerces does not ship a {@code TransformerFactory}, {@code XMLInputFactory} or - * {@code XPathFactory}, so this class only handles SAX and Schema factories. DOM hardening lives in {@link DocumentBuilderHardener}.
+ * {@code XPathFactory}, so this class only handles Schema factories; SAX hardening lives in {@link SAXParserHardener} and DOM hardening in + * {@link DocumentBuilderHardener}. * *Hardening recipe applied to every factory below uses the same building blocks:
*{@code HardeningXxx} wrappers + {@link Resolvers.DenyAll}: required. Xerces does not implement the JAXP 1.5 - * {@code ACCESS_EXTERNAL_*} properties, so an explicit resolver installed on every parser/validator is the best way to block external - * entity, DTD and schema fetching, without disabling those features altogether. The wrappers exist for two reasons:
- *Only {@link StreamSource} and {@link SAXSource} without a reader are enriched with a hardened reader. Other kinds of sources are returned as-is.
* + *The reader is namespace-aware.
+ * * @param source the source to harden; never {@code null}. * @return a hardened source. * @throws TransformerConfigurationException if a hardened reader cannot be obtained. @@ -149,7 +138,9 @@ private static SchemaFactory dispatch(final SchemaFactory factory) { public static Source harden(final Source source) throws TransformerConfigurationException { if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { try { - final XMLReader reader = newSAXParserFactory().newSAXParser().getXMLReader(); + final SAXParserFactory factory = newSAXParserFactory(); + factory.setNamespaceAware(true); + final XMLReader reader = factory.newSAXParser().getXMLReader(); final InputSource inputSource = SAXSource.sourceToInputSource(source); return inputSource == null ? source : new SAXSource(reader, inputSource); } catch (final ParserConfigurationException | SAXException e) { @@ -164,21 +155,10 @@ public static Source harden(final Source source) throws TransformerConfiguration * * @param reader the reader to harden; never {@code null}. * @return a hardened reader. - * @throws IllegalStateException if the reader's concrete class is not recognized by any bundled hardening recipe, or if the matching recipe cannot apply - * its settings to it. + * @throws IllegalStateException if a required hardening setting cannot be applied to the underlying implementation. */ public static XMLReader harden(final XMLReader reader) { - switch (reader.getClass().getName()) { - case "com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser": - return StockJdkProvider.configure(reader); - case "org.apache.harmony.xml.ExpatReader": - case "org.apache.commons.xml.AndroidProvider$GuardedXMLReader": - return AndroidProvider.configure(reader); - case "org.apache.xerces.jaxp.SAXParserImpl$JAXPSAXParser": - return XercesProvider.configure(reader); - default: - throw noProvider(reader); - } + return SAXParserHardener.hardenReader(reader); } /** @@ -203,11 +183,10 @@ public static DocumentBuilderFactory newDocumentBuilderFactory() { * an {@code xi:include} element fails. * * @return a hardened factory. - * @throws IllegalStateException if the underlying JAXP implementation is not recognized by any bundled hardening recipe, or if the matching recipe cannot - * apply its settings to it. + * @throws IllegalStateException if a required hardening setting cannot be applied to the underlying implementation. */ public static SAXParserFactory newSAXParserFactory() { - return dispatch(SAXParserFactory.newInstance()); + return SAXParserHardener.harden(SAXParserFactory.newInstance()); } /** diff --git a/src/test/java/org/apache/commons/xml/AttackTestSupport.java b/src/test/java/org/apache/commons/xml/AttackTestSupport.java index b02dc069..1afc49eb 100644 --- a/src/test/java/org/apache/commons/xml/AttackTestSupport.java +++ b/src/test/java/org/apache/commons/xml/AttackTestSupport.java @@ -732,7 +732,7 @@ private static SAXSource permissiveSaxSource(final String xml) { factory.setNamespaceAware(true); final XMLReader reader = strictXMLReader(factory); suppressException(() -> reader.setProperty(JDK_ENTITY_EXPANSION_LIMIT, "0")); - return new SAXSource(IS_ANDROID ? new AndroidProvider.GuardedXMLReader(reader) : reader, new InputSource(new StringReader(xml))); + return new SAXSource(IS_ANDROID ? new SAXParserHardener.ExpatReaderWrapper(reader) : reader, new InputSource(new StringReader(xml))); } private static boolean probeAndroid() { @@ -854,7 +854,7 @@ private static XMLReader strictXMLReader(final SAXParserFactory factory) { /** * Installs {@link #STRICT_REPORTER} as the error handler on {@code reader} and returns it; for raw-reader paths. */ - private static XMLReader strictXMLReader(final XMLReader reader) { + static XMLReader strictXMLReader(final XMLReader reader) { reader.setErrorHandler(STRICT_REPORTER); return reader; } diff --git a/src/test/java/org/apache/commons/xml/SchemaLocationSaxTest.java b/src/test/java/org/apache/commons/xml/SchemaLocationSaxTest.java new file mode 100644 index 00000000..6939817f --- /dev/null +++ b/src/test/java/org/apache/commons/xml/SchemaLocationSaxTest.java @@ -0,0 +1,128 @@ +/* + * 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.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.Assumptions.assumeTrue; + +import javax.xml.XMLConstants; +import javax.xml.parsers.SAXParser; +import javax.xml.parsers.SAXParserFactory; + +import org.junit.jupiter.api.Tag; +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; + +/** + * Checks that a hardened {@link SAXParserFactory} performing JAXP 1.2 XSD validation does not fetch an external schema named by an + * {@code xsi:noNamespaceSchemaLocation} hint in the instance document. + * + *This is the SAX counterpart of {@link SchemaLocationDomTest}. The instance is empty {@code
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.
+ */ +@Tag("sax") +class SchemaLocationSaxTest { + + /** Captures the root element's schema-defaulted {@code leak} attribute, the SAX-visible signal that the external schema was fetched. */ + private static final class LeakCapturingHandler extends DefaultHandler { + private String leak; + + @Override + public void startElement(final String uri, final String localName, final String qName, final Attributes attributes) { + if ("root".equals(localName) || "root".equals(qName)) { + leak = attributes.getValue("leak"); + } + } + } + + /** JAXP 1.2 property selecting the schema language used by {@link SAXParserFactory#setValidating(boolean)}. */ + private static final String SCHEMA_LANGUAGE = "http://java.sun.com/xml/jaxp/properties/schemaLanguage"; + + 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 { + 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()); + } + + @Test + void unconfiguredFetchesExternalSchema() throws Exception { + assumeTrue(supportsSchemaLanguage(), "parser does not support JAXP 1.2 schema-language XSD validation"); + final SAXParserFactory factory = SAXParserFactory.newInstance(); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, false); + final SAXParser parser = newValidatingParser(factory); + // Positive control: without hardening the external schema is fetched and its default attribute is augmented onto the root element. + final LeakCapturingHandler handler = new LeakCapturingHandler(); + parse(parser, handler); + assertEquals(LEAKED_MARKER, handler.leak, + "Permissive parse should have fetched the external schema and augmented its default attribute onto the element."); + } + + private static SAXParser newValidatingParser(final SAXParserFactory factory) throws Exception { + factory.setNamespaceAware(true); + factory.setValidating(true); + final SAXParser parser = factory.newSAXParser(); + parser.setProperty(SCHEMA_LANGUAGE, XMLConstants.W3C_XML_SCHEMA_NS_URI); + return parser; + } + + 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 + // 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); + AttackTestSupport.strictXMLReader(reader); + reader.parse(new InputSource(resourceUrl(INSTANCE).toString())); + } + + private static boolean supportsSchemaLanguage() { + try { + final SAXParserFactory factory = SAXParserFactory.newInstance(); + factory.setNamespaceAware(true); + factory.setValidating(true); + factory.newSAXParser().setProperty(SCHEMA_LANGUAGE, XMLConstants.W3C_XML_SCHEMA_NS_URI); + return true; + } catch (final Exception e) { + return false; + } + } +} diff --git a/src/test/java/org/apache/commons/xml/UnsupportedXmlImplementationTest.java b/src/test/java/org/apache/commons/xml/UnsupportedXmlImplementationTest.java index ff8d80a2..a6f7aa6f 100644 --- a/src/test/java/org/apache/commons/xml/UnsupportedXmlImplementationTest.java +++ b/src/test/java/org/apache/commons/xml/UnsupportedXmlImplementationTest.java @@ -24,8 +24,12 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParser; +import javax.xml.parsers.SAXParserFactory; import org.junit.jupiter.api.Test; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; /** * Verifies that an implementation which does not honour the required secure-processing feature surfaces {@link IllegalStateException} with a message naming the @@ -64,6 +68,27 @@ public boolean getFeature(final String name) { } } + /** + * A stand-in SAX factory that rejects the secure-processing feature, like a JAXP implementation that does not recognize it. + */ + public static final class FakeSAXParserFactory extends SAXParserFactory { + + @Override + public SAXParser newSAXParser() { + throw new UnsupportedOperationException(); + } + + @Override + public void setFeature(final String name, final boolean value) throws SAXNotRecognizedException { + throw new SAXNotRecognizedException("feature not recognized: " + name); + } + + @Override + public boolean getFeature(final String name) throws SAXNotSupportedException { + throw new SAXNotSupportedException("feature not recognized: " + name); + } + } + @Test void hardenRejectsUnsecurableFactory() { final IllegalStateException thrown = assertThrows( @@ -73,4 +98,14 @@ void hardenRejectsUnsecurableFactory() { assertTrue(thrown.getMessage().contains(FakeDocumentBuilderFactory.class.getName()), "Exception message must name the unsupported class: " + thrown.getMessage()); } + + @Test + void hardenRejectsUnsecurableSaxFactory() { + final IllegalStateException thrown = assertThrows( + IllegalStateException.class, + () -> SAXParserHardener.harden(new FakeSAXParserFactory())); + assertNotNull(thrown.getMessage()); + assertTrue(thrown.getMessage().contains(FakeSAXParserFactory.class.getName()), + "Exception message must name the unsupported class: " + thrown.getMessage()); + } }