From 514de3074ddd70db113931b06a0f0aaa8f6769d4 Mon Sep 17 00:00:00 2001 From: alhudz Date: Tue, 9 Jun 2026 16:25:17 +0530 Subject: [PATCH 1/2] harden ST_GeomFromGML against external entity expansion --- .../calcite/runtime/SpatialTypeUtils.java | 22 +++++++++++-- .../calcite/runtime/SpatialTypeUtilsTest.java | 31 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java b/core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java index f23514370e5a..cde9c79c914c 100644 --- a/core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java +++ b/core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java @@ -31,14 +31,19 @@ import org.locationtech.jts.io.WKTWriter; import org.locationtech.jts.io.geojson.GeoJsonReader; import org.locationtech.jts.io.geojson.GeoJsonWriter; -import org.locationtech.jts.io.gml2.GMLReader; +import org.locationtech.jts.io.gml2.GMLHandler; import org.locationtech.jts.io.gml2.GMLWriter; +import org.xml.sax.InputSource; import org.xml.sax.SAXException; +import org.xml.sax.XMLReader; import java.io.IOException; +import java.io.StringReader; import java.util.Locale; import java.util.regex.Pattern; import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParser; +import javax.xml.parsers.SAXParserFactory; import static java.lang.Integer.parseInt; @@ -135,8 +140,19 @@ public static Geometry fromGeoJson(String geoJson) { */ public static Geometry fromGml(String gml) { try { - GMLReader reader = new GMLReader(); - return reader.read(gml, GEOMETRY_FACTORY); + // GMLReader.read builds its own SAXParserFactory with external entities + // enabled, so parse with a hardened reader and feed JTS's GMLHandler. + final SAXParserFactory factory = SAXParserFactory.newInstance(); + factory.setNamespaceAware(true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + final SAXParser parser = factory.newSAXParser(); + final XMLReader xmlReader = parser.getXMLReader(); + final GMLHandler handler = new GMLHandler(GEOMETRY_FACTORY, null); + xmlReader.setContentHandler(handler); + xmlReader.parse(new InputSource(new StringReader(gml))); + return handler.getGeometry(); } catch (SAXException | IOException | ParserConfigurationException e) { throw new RuntimeException("Unable to parse GML"); } diff --git a/core/src/test/java/org/apache/calcite/runtime/SpatialTypeUtilsTest.java b/core/src/test/java/org/apache/calcite/runtime/SpatialTypeUtilsTest.java index 9789f0703295..34589c8b9a0c 100644 --- a/core/src/test/java/org/apache/calcite/runtime/SpatialTypeUtilsTest.java +++ b/core/src/test/java/org/apache/calcite/runtime/SpatialTypeUtilsTest.java @@ -21,8 +21,13 @@ import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * Tests {@link org.apache.calcite.runtime.SpatialTypeUtilsTest}. @@ -47,6 +52,32 @@ class SpatialTypeUtilsTest { assertThat(g3.getSRID(), is(0)); } + @Test void testFromGml() { + Geometry g = SpatialTypeUtils.fromGml( + "" + + "1,2"); + assertThat(g.getCoordinate().getX(), is(1D)); + assertThat(g.getCoordinate().getY(), is(2D)); + } + + /** A GML document declaring an external entity must be rejected, not have the + * entity resolved and its target file inlined into the geometry. The secret + * file holds a valid coordinate so an unguarded parser would parse it and + * succeed; the doctype guard makes parsing fail instead. */ + @Test void testFromGmlRejectsExternalEntities() throws Exception { + Path secret = Files.createTempFile("calcite-gml-xxe", ".txt"); + try { + Files.write(secret, "7".getBytes(StandardCharsets.UTF_8)); + String gml = "" + + " ]>" + + "" + + "&xxe;,8"; + assertThrows(RuntimeException.class, () -> SpatialTypeUtils.fromGml(gml)); + } finally { + Files.deleteIfExists(secret); + } + } + @Test void testAsEwkt() { GeometryFactory gf = new GeometryFactory(); Geometry g1 = gf.createPoint(new Coordinate(1, 2)); From 5a3d49274613ecedd26d35367a6690285b02426e Mon Sep 17 00:00:00 2001 From: alhudz Date: Tue, 9 Jun 2026 23:32:00 +0530 Subject: [PATCH 2/2] Drop redundant external-entity features, disallow-doctype-decl suffices --- .../java/org/apache/calcite/runtime/SpatialTypeUtils.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java b/core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java index cde9c79c914c..aa27321108c4 100644 --- a/core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java +++ b/core/src/main/java/org/apache/calcite/runtime/SpatialTypeUtils.java @@ -140,13 +140,12 @@ public static Geometry fromGeoJson(String geoJson) { */ public static Geometry fromGml(String gml) { try { - // GMLReader.read builds its own SAXParserFactory with external entities - // enabled, so parse with a hardened reader and feed JTS's GMLHandler. + // GMLReader.read builds its own SAXParserFactory with DOCTYPE enabled, so + // parse with a hardened reader and feed JTS's GMLHandler. Disallowing the + // DOCTYPE declaration rejects any external subset or entities outright. final SAXParserFactory factory = SAXParserFactory.newInstance(); factory.setNamespaceAware(true); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); final SAXParser parser = factory.newSAXParser(); final XMLReader xmlReader = parser.getXMLReader(); final GMLHandler handler = new GMLHandler(GEOMETRY_FACTORY, null);