[ppkarwasz]

Alternative to #1221, same problem, different fix.

Problem

GMLReader.read() builds its SAXParserFactory with only setNamespaceAware(false) and setValidating(false), so DOCTYPE processing and external entity resolution stay enabled. GML is routinely read from untrusted input, so a crafted document can disclose local files or trigger SSRF through an external entity (XXE):

<!DOCTYPE foo [ <!ENTITY xxe SYSTEM "file:///etc/hostname"> ]>
<gml:Point><gml:coordinates>&xxe;</gml:coordinates></gml:Point>

KMLReader received the equivalent hardening in #1204.

Fix

Create the factory through SecureSAXParserFactory.newInstance() from Apache Commons Secure XML instead of SAXParserFactory.newInstance(). Every factory the library returns makes the same three guarantees regardless of which JAXP implementation is on the classpath:

• external DTDs are not fetched,
• external entities are not resolved,
• internal entity expansion is bounded (secure processing plus the implementation's limits, so Billion Laughs is rejected).

Compared with #1221, which sets Xerces-specific feature names by hand and logs a warning when the parser does not recognize them, the library either secures the factory or fails closed; there is no silent fallback to an unhardened parser. The reader keeps setNamespaceAware(false) / setValidating(false) (it deliberately accepts unbound gml: prefixes), keeps its signatures, and parses valid GML exactly as before.

The class Javadoc now says where the parser comes from and links to the library's package documentation and threat model, so security scanners have something to point at instead of flagging the reader.

Points for the reviewers

• First runtime dependency of jts-core. org.apache.commons:commons-secure-xml is a ~75 KB OSGi bundle with no transitive dependencies, compatible with Java 8; maven-bundle-plugin imports it as org.apache.commons.xml.secure;version="[1.0,2)" without any manifest change. If an external dependency is not acceptable, the library is designed to be shaded: with minimizeJar, the SAX entry point adds about 10 kB to jts-core, and JTS still picks up fixes by bumping the version.
• No new tests, on purpose (see Verification): they would only re-test another library's contract.
• Javadoc autolinking. The PR adds a maven-javadoc-plugin <links> entry to https://javadoc.io (unresolvable until 1.0.0 is published) so that {@link SecureSAXParserFactory} in the GMLReader Javadoc becomes a hyperlink, and automated tools following it land on the documented guarantees rather than reporting a false positive. I can switch it to the Javadoc on https://commons.apache.org, which always shows the latest version (a plus or a minus depending on the shading decision above), or drop the autolinking entirely.

Verification

As noted above, there are no new XXE unit tests: the guarantees belong to the library and are tested there. Instead, a forbidden-apis check now fails the jts-core build whenever a raw SAX factory method is used (SAXParserFactory.newInstance/newDefaultInstance/newNSInstance overloads and XMLReaderFactory.createXMLReader). Running it against the unpatched reader:

[ERROR] Forbidden method invocation: javax.xml.parsers.SAXParserFactory#newInstance() [Create SAX parsers through org.apache.commons.xml.secure.SecureSAXParserFactory, which never resolves external DTDs or entities]
[ERROR]   in org.locationtech.jts.io.gml2.GMLReader (GMLReader.java:111)
[ERROR] Scanned 758 class file(s) for forbidden API invocations (in 0.12s), 1 error(s).

mvn clean install on Temurin 8 passes (jts-core: 2296 tests, 0 failures).

🤖 Generated with Claude Code

[ppkarwasz]

One more design choice worth considering:

Commons Secure XML also backports newDefaultInstance() from JDK 9, so GMLReader could create its parser with SecureSAXParserFactory.newDefaultInstance() instead of newInstance(). That pins the reader to the JDK's built-in parser and opts out of JAXP pluggability, so a stray Xerces on the classpath can no longer change how JTS parses GML. Users who need a different parser already have that option: GMLHandler lets them drive any SAX parser of their own.

[jodygarnett]

Hi @ppkarwasz this has a format similar to a security vulnerability report; which is a different workflow.

This also appears to be a duplicate of #1221 but I will confirm that.

[ppkarwasz]

Hi @bjornharrtell,

Yes, this is an alternative to #1221. It might have security implications for downstream users, allowing them to parse untrusted data through GMLReader directly, instead of using GMLHandler.

As far as I could check, this library never specified if it can be used for that purpose.

This approach differs from #1221, because it delegates the security aspects to Commos Secure XML and allows JTS to concentrate on the rest.

[bjornharrtell]

@ppkarwasz why are you pinging me?

[ppkarwasz]

Sorry, fat fingers, I meant to ping @jodygarnett . 😉

[jodygarnett]

Hi @bjornharrtell,

Yes, this is an alternative to #1221. It might have security implications for downstream users, allowing them to parse untrusted data through GMLReader directly, instead of using GMLHandler.

As far as I could check, this library never specified if it can be used for that purpose.

This approach differs from #1221, because it delegates the security aspects to Commos Secure XML and allows JTS to concentrate on the rest.

Thanks for the clarification, we really wish to keep this library simple and avoid adding dependencies.

Let me review #1221 and report back.

[jodygarnett]

Okay I have a simple #1244 prepared as an alternative. Feedback welcome.