From c9ab87793bd75ad0a876cd81e74133018a27efb6 Mon Sep 17 00:00:00 2001 From: Nexory Date: Thu, 6 Aug 2026 15:46:04 +0200 Subject: [PATCH] Harden GMLReader against XXE (disable DTDs and external entities) GMLReader configured the SAX parser with only namespace-awareness and validation disabled, leaving DOCTYPE processing and external entity resolution enabled. GML is commonly read from untrusted sources (files, WFS responses, uploads), so a crafted document could disclose local files or trigger SSRF via an external entity (XXE). Enable JAXP secure processing and disable DTDs and external entities on the SAXParserFactory. There is no behaviour change for valid GML, and no signature change (setFeature only throws SAXException subclasses, which are already declared). This mirrors the KMLReader hardening in #1204. Adds GMLReaderXXETest: without the fix the external entity is resolved and a DOCTYPE is accepted; with it both are rejected and benign GML still parses. Signed-off-by: Nexory --- .../locationtech/jts/io/gml2/GMLReader.java | 9 ++ .../jts/io/gml2/GMLReaderXXETest.java | 82 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 modules/core/src/test/java/org/locationtech/jts/io/gml2/GMLReaderXXETest.java diff --git a/modules/core/src/main/java/org/locationtech/jts/io/gml2/GMLReader.java b/modules/core/src/main/java/org/locationtech/jts/io/gml2/GMLReader.java index 7bef25880f..0fee770eb7 100644 --- a/modules/core/src/main/java/org/locationtech/jts/io/gml2/GMLReader.java +++ b/modules/core/src/main/java/org/locationtech/jts/io/gml2/GMLReader.java @@ -15,6 +15,7 @@ import java.io.Reader; import java.io.StringReader; +import javax.xml.XMLConstants; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -105,6 +106,14 @@ public Geometry read(Reader reader, GeometryFactory geometryFactory) throws SAXE fact.setNamespaceAware(false); fact.setValidating(false); + // Harden against XXE: disable DOCTYPE/DTDs and external entities (JAXP secure processing). + // GML input is frequently untrusted (files, WFS responses, uploads); the default SAX parser + // resolves external entities, enabling file disclosure and SSRF. + fact.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + fact.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + fact.setFeature("http://xml.org/sax/features/external-general-entities", false); + fact.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + fact.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); SAXParser parser = fact.newSAXParser(); diff --git a/modules/core/src/test/java/org/locationtech/jts/io/gml2/GMLReaderXXETest.java b/modules/core/src/test/java/org/locationtech/jts/io/gml2/GMLReaderXXETest.java new file mode 100644 index 0000000000..a8481c5e70 --- /dev/null +++ b/modules/core/src/test/java/org/locationtech/jts/io/gml2/GMLReaderXXETest.java @@ -0,0 +1,82 @@ +package org.locationtech.jts.io.gml2; + +import java.io.File; +import java.nio.file.Files; + +import org.locationtech.jts.geom.Geometry; + +import junit.textui.TestRunner; +import test.jts.GeometryTestCase; + +/** + * Tests that {@link GMLReader} does not resolve external XML entities (XXE). + * GML is frequently read from untrusted sources (files, WFS responses, uploads), + * so the underlying SAX parser must not fetch external entities or process DTDs. + */ +public class GMLReaderXXETest extends GeometryTestCase { + + public static void main(String[] args) { + TestRunner.run(GMLReaderXXETest.class); + } + + public GMLReaderXXETest(String name) { + super(name); + } + + /** + * An external general entity referencing a local file must not be resolved. + * Without the hardening the file content leaks into the parse (and, here, into + * the exception raised while parsing it as a coordinate); with it, the DOCTYPE + * is rejected before any entity is resolved. + */ + public void testExternalEntityIsNotResolved() throws Exception { + File secretFile = File.createTempFile("jts-xxe", ".txt"); + String secret = "JTS-XXE-CANARY-SECRET"; + Files.write(secretFile.toPath(), secret.getBytes("UTF-8")); + try { + String gml = + "\n" + + " ]>\n" + + "&xxe;"; + try { + new GMLReader().read(gml, null); + } + catch (Exception e) { + // The parser may legitimately reject the input; it must never expose the + // external file's content (which would prove the entity was resolved). + assertFalse("GMLReader resolved an external entity (XXE): " + e.getMessage(), + String.valueOf(e.getMessage()).contains(secret)); + } + } + finally { + secretFile.delete(); + } + } + + /** + * A DOCTYPE declaration must be rejected outright (billion-laughs / DTD surface). + */ + public void testDoctypeIsRejected() throws Exception { + String gml = + "\n" + + "\n" + + "5,10"; + try { + new GMLReader().read(gml, null); + fail("expected a DOCTYPE to be rejected"); + } + catch (Exception e) { + // expected: parser refuses the DOCTYPE + } + } + + /** + * Legitimate GML without a DOCTYPE must still parse unchanged. + */ + public void testBenignGmlStillParses() throws Exception { + Geometry g = new GMLReader().read( + "5,10", null); + assertNotNull(g); + assertEquals("POINT (5 10)", g.toText()); + } +}