ppkarwasz opened a new pull request, #46:
URL: https://github.com/apache/commons-xml/pull/46

   Closes the last gap from the internal-parser audit (G2): 
`getAssociatedStylesheet` on Apache Xalan parsed the PI scan with a reader the 
engine provisions itself, ignoring the hardened reader passed in a `SAXSource`.
   
   ## Problem
   
   `getAssociatedStylesheet` hardens its argument through 
`SAXParserHardener.hardenSource`, which for a reader-less source produces a 
`SAXSource` carrying a hardened reader. The JDK's XSLTC honors that reader, but 
Apache Xalan's 
`org.apache.xalan.processor.TransformerFactoryImpl.getAssociatedStylesheet` 
drops it and builds its own reader to scan for `xml-stylesheet` PIs (filed 
upstream as [XALANJ-2849](https://issues.apache.org/jira/browse/XALANJ-2849)). 
Under FSP that reader disables external general entities but still processes 
the external DTD subset and external parameter entities during the prolog 
parse, so a document scanned on Xalan can trigger an external 
DTD/parameter-entity fetch (SSRF) or expansion.
   
   ## Change
   
   When the delegate is Xalan (package prefix `org.apache.xalan.`), 
`getAssociatedStylesheet` now pre-parses a reader-less source through a 
hardened, namespace-aware `DocumentBuilder` and hands the delegate a 
`DOMSource`, which both the interpretive and XSLTC Xalan factories walk via a 
tree walker without provisioning any internal reader. The JDK's XSLTC 
(`com.sun.org.apache.xalan.internal.*`) honors the `SAXSource` reader, so it 
keeps the streaming SAX path unchanged and does not pay the cost of 
materializing a full DOM just to work around a Xalan bug. 
`HardeningTransformerFactory` only wraps XSLTC or Xalan (Saxon takes the 
`SaxonProvider` Configuration path), so the branch is exactly Xalan → 
DOMSource, else → `hardenSource`. Only `getAssociatedStylesheet` changes; the 
other Source-taking entry points keep the streaming SAX rewrite.
   
   The `DocumentBuilderHardener` reference is static, so it grows the 
TransformerHardener **shade footprint** by three classes (shipping size, not 
per-parse memory). On the Xalan path the scanned document must now be fully 
well-formed (the SAX PI-scan previously tolerated malformedness after the root 
element).
   
   ## Tests
   
   New `AssociatedStylesheetTest` (`@Tag("trax")`), run on the stock JDK, 
Apache Xalan, Saxon, and the Android runtime:
   
   - `hardenedGetAssociatedStylesheetIgnoresExternalDtd`: the hardened lookup 
on a document whose prolog declares an unreachable external DTD completes and 
finds the PI, rather than throwing on a fetch (the returned Source's shape is 
engine-specific: XSLTC/Xalan point it at the stylesheet, Saxon resolves the 
href through its own floor to an empty source; both mean the DTD was not 
fetched).
   - `hardenedGetAssociatedStylesheetReturnsStylesheet`: positive control on a 
plain document.
   - `unconfiguredGetAssociatedStylesheetFetchesExternalDtd`: leak control — 
the unconfigured engine attempts the fetch and throws (skipped on Android, 
whose KXmlParser does not fetch external DTDs).
   
   Discrimination verified: with the Xalan branch forced back to 
`hardenSource`, the security test errors only on `test-xalan` / 
`test-xalan-xerces` (Xalan then drops the reader, self-parses, and throws 
fetching the missing DTD), while stock JDK and Saxon stay green. 
`ShadingFootprintTest` gains the DOM hardener set in the transformer closure. 
Full `mvn clean` + `mvn` (checkstyle, spotbugs, pmd, javadoc, whole surefire 
matrix) green, plus the Android instrumented suite (`connectedAndroidTest`, 
Pixel 6a API 33): 123 tests, 0 failures, both hardened cases passing on the 
Harmony/KXmlParser DOM path.
   
   ## Note
   
   Open PR #41 rewrites `HardeningTransformerFactory` substantially and open PR 
#44 makes the same TransformerHardener-footprint DOM change on the XPath side; 
whichever lands later reconciles the `getAssociatedStylesheet` body and the 
`ShadingFootprintTest` transformer set.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to