[CALCITE-7601] Harden ST_GeomFromGML against external entity expansion

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,18 @@ 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 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);
+      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");
     }

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 @@
     assertThat(g3.getSRID(), is(0));
   }
 
+  @Test void testFromGml() {
+    Geometry g = SpatialTypeUtils.fromGml(
+        "<gml:Point xmlns:gml=\"http://www.opengis.net/gml\">"
+        + "<gml:coordinates>1,2</gml:coordinates></gml:Point>");
+    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 = "<?xml version=\"1.0\"?>"
+          + "<!DOCTYPE gml [ <!ENTITY xxe SYSTEM \"" + secret.toUri() + "\"> ]>"
+          + "<gml:Point xmlns:gml=\"http://www.opengis.net/gml\">"
+          + "<gml:coordinates>&xxe;,8</gml:coordinates></gml:Point>";
+      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));