ppkarwasz commented on code in PR #56:
URL: https://github.com/apache/commons-xml/pull/56#discussion_r3880004870
##########
src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java:
##########
@@ -41,6 +48,108 @@
*/
public final class HardeningSAXParserFactory {
+ /** Class name of Android's Expat-backed {@link XMLReader}. */
+ private static final String ANDROID_EXPAT_READER =
"org.apache.harmony.xml.ExpatReader";
+ /** 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";
+
+ /**
+ * Capability-driven hardening for any {@link SAXParserFactory} on the
classpath.
+ *
+ * <p>Rather than branching on the implementation class, this method
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 the nested wrapper into {@link
#harden(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: 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>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>
+ * </ul>
+ *
+ * @param factory the factory to harden; never {@code null}.
+ * @return a hardened factory.
+ */
+ 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 harden(XMLReader) because SAXParserFactory has no property API.
+ return new Wrapper(factory);
+ }
+
+ /**
+ * Rewrites a {@link Source} so that any SAX parsing it triggers runs
through a hardened {@link XMLReader}.
+ * <p>
+ * Only a {@link StreamSource} or a {@link SAXSource} without a reader is
enriched with a hardened, namespace-aware reader; other source kinds are
returned
+ * as-is. Used by the TrAX and schema wrappers to route every source they
parse through the SAX hardening path.
+ * </p>
+ *
+ * @param source the source to harden; never {@code null}.
+ * @return a hardened source.
+ * @throws TransformerConfigurationException if a hardened reader cannot
be obtained.
+ * @throws FactoryConfigurationError 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.
+ */
+ static Source harden(final Source source) throws
TransformerConfigurationException {
+ if (source instanceof StreamSource || source instanceof SAXSource &&
((SAXSource) source).getXMLReader() == null) {
+ final InputSource inputSource =
SAXSource.sourceToInputSource(source);
+ return inputSource == null ? source : new
SAXSource(newHardenedReader(), inputSource);
+ }
+ return source;
+ }
+
+ /**
+ * 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 harden(final XMLReader reader) {
+ if (reader instanceof HardeningXMLReader) {
+ // Already hardened (for example, a reader from a hardened factory
passed back through harden(XMLReader)); the floor is already in place.
+ return reader;
+ }
+ if (ANDROID_EXPAT_READER.equals(reader.getClass().getName())) {
+ // 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);
+ // 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);
+ }
+
+ /**
+ * Creates a new hardened, namespace-aware {@link XMLReader} for the TrAX
wrappers to parse sources with.
+ *
+ * @return a hardened reader.
+ * @throws TransformerConfigurationException if a hardened reader cannot
be obtained.
+ * @throws FactoryConfigurationError 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.
+ */
+ static XMLReader newHardenedReader() throws
TransformerConfigurationException {
Review Comment:
Good point, go ahead! :100:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
