Make XML parsing and validation resilient to outdated XML parsers on the classpath - #894
Conversation
…the classpath When an outdated XML parser such as Xerces 2.x leaks onto the classpath (pulled in transitively by another library), the JAXP lookup mechanism picks it up and BOM parsing/validation fails with errors like: SAXNotRecognizedException: Property 'http://javax.xml.XMLConstants/property/accessExternalDTD' is not recognized. because such parsers pre-date the JAXP 1.5 secure-processing properties. Reported against the Gradle plugin in CycloneDX/cyclonedx-gradle-plugin#349, where any plugin leaking Xerces onto the buildscript classpath breaks BOM generation. - introduce XmlFactoryUtils that prefers the JDK built-in SchemaFactory/DocumentBuilderFactory (newDefaultInstance, invoked reflectively for Java 8 compatibility) over the classpath-based JAXP lookup - use it in CycloneDxSchema, XmlParser and BomXmlGenerator - treat the ACCESS_EXTERNAL_DTD/SCHEMA hardening properties as best-effort for exotic factory implementations - add xercesImpl to the test classpath as regression coverage: before this change, all XmlParserTest cases fail with the error above Signed-off-by: Patrick Baumgartner <contact@patbaumgartner.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 23 |
🟢 Coverage 82.69% diff coverage · +0.02% coverage variation
Metric Results Coverage variation ✅ +0.02% coverage variation Diff coverage ✅ 82.69% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (65c2eee) Report Missing Report Missing Report Missing Head commit (af3ad2e) 7751 (+43) 5799 (+34) 74.82% (+0.02%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#894) 52 43 82.69% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…path in CI Address review feedback: - disallow DOCTYPE declarations when the JAXP 1.5 secure-processing properties are unsupported, failing instead of running insecurely - the disallow-doctype-decl feature set on Xerces' XMLSchemaFactory is accepted but not propagated to Validator instances, leaving validation vulnerable to XXE: probe the Validator for the JAXP 1.5 properties and, when unsupported, validate a DOM parsed by the hardened DocumentBuilder - honor explicit JAXP system properties in XmlFactoryUtils (only the accidental classpath lookup is bypassed) and use them in the new XercesFallbackTest to instantiate real Xerces factories on any JVM, covering parse, validate, generate, and both XXE rejection paths in CI Signed-off-by: Patrick Baumgartner <contact@patbaumgartner.com>
2f5c78f to
33fa524
Compare
Address review feedback: - instead of materializing a DOM, wrap the input in a SAXSource backed by a hardened XMLReader when the Validator does not support the JAXP 1.5 properties: validation stays streaming for large BOMs - SAXSource inputs are re-wrapped with the hardened reader as well; DOMSource/StAXSource carry pre-parsed content and pass through - new XmlFactoryUtils.newSAXParserFactory follows the same lookup policy as the other factories - XercesFallbackTest uses @SetSystemProperty and additionally forces Xerces' SAXParserFactory so the streaming path runs against a real outdated SAX parser Signed-off-by: Patrick Baumgartner <contact@patbaumgartner.com>
Related to CycloneDX/cyclonedx-gradle-plugin#349
Problem
When an outdated XML parser such as Xerces 2.x leaks onto the classpath (pulled in transitively by another library or Gradle plugin), the JAXP lookup mechanism (
SchemaFactory.newInstance/DocumentBuilderFactory.newInstance) picks it up, and BOM parsing/validation fails hard:These parsers pre-date the JAXP 1.5 secure-processing properties. This is what breaks
cyclonedxBomin CycloneDX/cyclonedx-gradle-plugin#349 whenever any other plugin ships Xerces on the buildscript classpath - reproducible with a plain Gradle 8.4 project, the current gradle plugin 3.3.0, andxerces:xercesImpl:2.12.2added to the buildscript classpath.Fix
XmlFactoryUtilsprefers the JDK's built-inSchemaFactory/DocumentBuilderFactoryvianewDefaultInstance(), bypassing the classpath-based JAXP lookup. An implementation explicitly requested via the JAXP system properties is still honored - only the accidental classpath/ServiceLoader leak is bypassed.newDefaultInstance()only exists since Java 9 while this library targets Java 8, so it is invoked reflectively with a fallback to the standard lookupCycloneDxSchema.getXmlSchema,XmlParser.createSecureDocument,BomXmlGenerator.buildSecureDocumentBuilderXXE compensations when an outdated parser is chosen (review feedback)
Secure processing alone does not prevent XXE in Xerces 2.x, so when the JAXP 1.5
ACCESS_EXTERNAL_DTD/ACCESS_EXTERNAL_SCHEMAproperties are unsupported the code now compensates instead of continuing insecurely:CycloneDxSchema.getXmlSchemaandXmlParser.createSecureDocumentsethttp://apache.org/xml/features/disallow-doctype-declas a fallback; if that is unsupported too, the exception propagates (fail-secure)disallow-doctype-declonXMLSchemaFactorybut does not propagate it toValidatorinstances (XMLSchemaValidatorComponentManagerhardcodes it tofalse), leaving validation of untrusted input XXE-vulnerable.XmlParser.validatetherefore probes theValidatorfor the JAXP 1.5 properties and, when unsupported, wraps the input in aSAXSourcebacked by a hardenedXMLReader- validation stays fully streaming, andSAXSourceinputs are re-wrapped the same way (DOMSource/StAXSourcecarry pre-parsed content and pass through)Regression coverage
CI runs on Java 17/21/25 where
newDefaultInstance()always succeeds, so a leaked Xerces would never be instantiated by the tests. The newXercesFallbackTestforces real Xerces 2.12.2 factories via the explicit JAXP system properties (guarded by an assertion that the factories actually are Xerces) and verifies on every CI JVM:security/xxe-protection.xml) is rejected withDOCTYPE is disallowedon both the parse and the validate pathVerification
mvn clean verifygreen (1620 tests, 0 failures)xercesImplon the buildscript classpath fails withError whilst validating XML BOM/SAXNotRecognizedExceptiontoday; with this fix in core-java, validation uses the JDK parser and is unaffected