CVE-2024-36404

@@ -18,10 +18,10 @@
 package org.geotools.appschema.util;
 
 import java.util.ArrayList;
-import java.util.Enumeration;
 import java.util.List;
 import java.util.Scanner;
 import org.apache.commons.jxpath.JXPathContext;
+import org.geotools.xsd.impl.jxpath.JXPathUtils;
 import org.jdom2.Document;
 import org.xml.sax.helpers.NamespaceSupport;
 
@@ -41,7 +41,7 @@ public class XmlXpathUtilites {
      */
     public static List<String> getXPathValues(
             NamespaceSupport ns, String xpathString, Document doc) {
-        JXPathContext context = initialiseContext(ns, doc);
+        JXPathContext context = JXPathUtils.newSafeContext(doc, true, ns, false);
         return getXPathValues(xpathString, context);
     }
 
@@ -69,33 +69,15 @@ public static int countXPathNodes(NamespaceSupport ns, String xpathString, Docum
     public static String getSingleXPathValue(
             NamespaceSupport ns, String xpathString, Document doc) {
         String id = null;
-        JXPathContext context = initialiseContext(ns, doc);
         try {
-            Object ob = context.getValue(xpathString);
+            Object ob = JXPathUtils.newSafeContext(doc, true, ns, false).getValue(xpathString);
             id = (String) ob;
         } catch (RuntimeException e) {
             throw new RuntimeException("Error reading xpath " + xpathString, e);
         }
         return id;
     }
 
-    private static JXPathContext initialiseContext(NamespaceSupport ns, Document doc) {
-        JXPathContext context = JXPathContext.newContext(doc);
-        addNamespaces(ns, context);
-        context.setLenient(true);
-        return context;
-    }
-
-    private static void addNamespaces(NamespaceSupport ns, JXPathContext context) {
-        @SuppressWarnings("unchecked")
-        Enumeration<String> prefixes = ns.getPrefixes();
-        while (prefixes.hasMoreElements()) {
-            String prefix = prefixes.nextElement();
-            String uri = ns.getURI(prefix);
-            context.registerNamespace(prefix, uri);
-        }
-    }
-
     /**
      * Remove indexes from an xpath string.
      *

@@ -0,0 +1,58 @@
+/*
+ *    GeoTools - The Open Source Java GIS Toolkit
+ *    http://geotools.org
+ *
+ *    (C) 2024, Open Source Geospatial Foundation (OSGeo)
+ *
+ *    This library is free software; you can redistribute it and/or
+ *    modify it under the terms of the GNU Lesser General Public
+ *    License as published by the Free Software Foundation;
+ *    version 2.1 of the License.
+ *
+ *    This library is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *    Lesser General Public License for more details.
+ */
+package org.geotools.appschema.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+import org.junit.Test;
+
+public class XmlXpathUtilitesTest {
+
+    @Test
+    public void testGetXPathValuesWithJavaMethod() {
+        RuntimeException exception =
+                assertThrows(
+                        RuntimeException.class,
+                        () ->
+                                XmlXpathUtilites.getXPathValues(
+                                        null, "java.lang.Thread.sleep(30000)", null));
+        assertEquals("Error reading xpath java.lang.Thread.sleep(30000)", exception.getMessage());
+    }
+
+    @Test
+    public void testCountXPathNodesWithJavaMethod() {
+        RuntimeException exception =
+                assertThrows(
+                        RuntimeException.class,
+                        () ->
+                                XmlXpathUtilites.countXPathNodes(
+                                        null, "java.lang.Thread.sleep(30000)", null));
+        assertEquals("Error reading xpath java.lang.Thread.sleep(30000)", exception.getMessage());
+    }
+
+    @Test
+    public void testGetSingleXPathValueWithJavaMethod() {
+        RuntimeException exception =
+                assertThrows(
+                        RuntimeException.class,
+                        () ->
+                                XmlXpathUtilites.getSingleXPathValue(
+                                        null, "java.lang.Thread.sleep(30000)", null));
+        assertEquals("Error reading xpath java.lang.Thread.sleep(30000)", exception.getMessage());
+    }
+}

@@ -20,7 +20,6 @@
 import static org.geotools.filter.expression.SimpleFeaturePropertyAccessorFactory.DEFAULT_GEOMETRY_NAME;
 
 import java.util.ArrayList;
-import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.List;
 import java.util.regex.Pattern;
@@ -33,6 +32,7 @@
 import org.geotools.api.feature.GeometryAttribute;
 import org.geotools.api.feature.IllegalAttributeException;
 import org.geotools.api.feature.simple.SimpleFeature;
+import org.geotools.api.feature.simple.SimpleFeatureType;
 import org.geotools.api.feature.type.AttributeDescriptor;
 import org.geotools.api.feature.type.AttributeType;
 import org.geotools.api.feature.type.ComplexType;
@@ -44,6 +44,7 @@
 import org.geotools.filter.expression.PropertyAccessor;
 import org.geotools.filter.expression.PropertyAccessorFactory;
 import org.geotools.util.factory.Hints;
+import org.geotools.xsd.impl.jxpath.JXPathUtils;
 import org.locationtech.jts.geom.Geometry;
 import org.xml.sax.helpers.NamespaceSupport;
 
@@ -83,7 +84,8 @@ public class FeaturePropertyAccessorFactory implements PropertyAccessorFactory {
     public PropertyAccessor createPropertyAccessor(
             Class type, String xpath, Class target, Hints hints) {
 
-        if (SimpleFeature.class.isAssignableFrom(type)) {
+        if (SimpleFeature.class.isAssignableFrom(type)
+                || SimpleFeatureType.class.isAssignableFrom(type)) {
             /*
              * This class is not intended for use with SimpleFeature and causes problems when
              * discovered via SPI and used by code expecting SimpleFeature behaviour. In particular
@@ -260,14 +262,8 @@ public boolean canHandle(Object object, String xpath, Class target) {
         public <T> T get(Object object, String xpath, Class<T> target)
                 throws IllegalArgumentException {
 
-            JXPathContext context = JXPathContext.newContext(object);
-            Enumeration declaredPrefixes = namespaces.getDeclaredPrefixes();
-            while (declaredPrefixes.hasMoreElements()) {
-                String prefix = (String) declaredPrefixes.nextElement();
-                String uri = namespaces.getURI(prefix);
-                context.registerNamespace(prefix, uri);
-            }
-
+            JXPathContext context =
+                    JXPathUtils.newSafeContext(object, false, this.namespaces, true);
             Iterator it = context.iteratePointers(xpath);
             List results = new ArrayList<>();
             while (it.hasNext()) {
@@ -296,13 +292,8 @@ public void set(Object object, String xpath, Object value, Class target)
                 throw new IllegalAttributeException(null, "feature type is immutable");
             }
 
-            JXPathContext context = JXPathContext.newContext(object);
-            Enumeration declaredPrefixes = namespaces.getDeclaredPrefixes();
-            while (declaredPrefixes.hasMoreElements()) {
-                String prefix = (String) declaredPrefixes.nextElement();
-                String uri = namespaces.getURI(prefix);
-                context.registerNamespace(prefix, uri);
-            }
+            JXPathContext context =
+                    JXPathUtils.newSafeContext(object, false, this.namespaces, true);
             context.setValue(xpath, value);
 
             assert value == context.getValue(xpath);

@@ -18,11 +18,11 @@
 package org.geotools.data.complex.expression;
 
 import java.util.Map;
-import org.apache.commons.jxpath.JXPathContext;
 import org.geotools.api.feature.IllegalAttributeException;
 import org.geotools.filter.expression.PropertyAccessor;
 import org.geotools.filter.expression.PropertyAccessorFactory;
 import org.geotools.util.factory.Hints;
+import org.geotools.xsd.impl.jxpath.JXPathUtils;
 
 /**
  * A {@link PropertyAccessorFactory} that returns a {@link PropertyAccessor} capable of evaluating
@@ -63,9 +63,7 @@ public boolean canHandle(Object object, String xpath, Class target) {
                 @SuppressWarnings("unchecked")
                 public <T> T get(Object object, String xpath, Class<T> target)
                         throws IllegalArgumentException {
-                    JXPathContext context = JXPathContext.newContext(object);
-                    context.setLenient(true);
-                    return (T) context.getValue(xpath);
+                    return (T) JXPathUtils.newSafeContext(object, true).getValue(xpath);
                 }
 
                 @Override

@@ -290,6 +290,20 @@ public void testComplexFeature() {
                         "eg:complexAttribute/eg:rootAttribute/eg:multiLeafAttribute",
                         new Hints(FeaturePropertyAccessorFactory.NAMESPACE_CONTEXT, NAMESPACES));
         assertEquals(multiLeafDesc, ex.evaluate(fType));
+
+        // test that executing Java methods is blocked
+        ex =
+                new AttributeExpressionImpl(
+                        "java.lang.Thread.sleep(30000)",
+                        new Hints(FeaturePropertyAccessorFactory.NAMESPACE_CONTEXT, NAMESPACES));
+        long start = System.currentTimeMillis();
+        assertNull(ex.evaluate(feature));
+        long runtime = System.currentTimeMillis() - start;
+        assertTrue("java.lang.Thread.sleep(30000) was executed", runtime < 30000);
+        start = System.currentTimeMillis();
+        assertNull(ex.evaluate(fType));
+        runtime = System.currentTimeMillis() - start;
+        assertTrue("java.lang.Thread.sleep(30000) was executed", runtime < 30000);
     }
 
     public static FeatureType createFeatureType() {

@@ -0,0 +1,37 @@
+/*
+ *    GeoTools - The Open Source Java GIS Toolkit
+ *    http://geotools.org
+ *
+ *    (C) 2024, Open Source Geospatial Foundation (OSGeo)
+ *
+ *    This library is free software; you can redistribute it and/or
+ *    modify it under the terms of the GNU Lesser General Public
+ *    License as published by the Free Software Foundation;
+ *    version 2.1 of the License.
+ *
+ *    This library is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *    Lesser General Public License for more details.
+ */
+package org.geotools.filter.expression;
+
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collections;
+import org.geotools.api.filter.expression.PropertyName;
+import org.geotools.factory.CommonFactoryFinder;
+import org.junit.Test;
+
+public class MapPropertyAccessorTest {
+
+    @Test
+    public void testAccessWithJavaMethod() {
+        PropertyName property =
+                CommonFactoryFinder.getFilterFactory().property("java.lang.Thread.sleep(30000)");
+        long start = System.currentTimeMillis();
+        property.evaluate(Collections.emptyMap());
+        long runtime = System.currentTimeMillis() - start;
+        assertTrue("java.lang.Thread.sleep(30000) was executed", runtime < 30000);
+    }
+}

@@ -90,20 +90,6 @@
       <groupId>commons-jxpath</groupId>
       <artifactId>commons-jxpath</artifactId>
       <version>1.3</version>
-      <exclusions>
-        <exclusion>
-          <groupId>xerces</groupId>
-          <artifactId>xerces</artifactId>
-        </exclusion>
-        <exclusion>
-          <groupId>servletapi</groupId>
-          <artifactId>servletapi</artifactId>
-        </exclusion>
-        <exclusion>
-          <groupId>junit</groupId>
-          <artifactId>junit</artifactId>
-        </exclusion>
-      </exclusions>
     </dependency>
     <dependency>
       <groupId>org.apache.commons</groupId>

@@ -17,8 +17,6 @@
 package org.geotools.xsd.impl.jxpath;
 
 import java.util.Iterator;
-import org.apache.commons.jxpath.JXPathContext;
-import org.apache.commons.jxpath.JXPathContextFactory;
 import org.apache.commons.jxpath.JXPathIntrospector;
 import org.geotools.xsd.Configuration;
 import org.geotools.xsd.Node;
@@ -47,11 +45,7 @@ protected boolean stream(ElementHandler handler) {
         //        ElementHandler rootHandler =
         //        	((DocumentHandler) handlers.firstElement()).getDocumentElementHandler();
         Node root = handlers.firstElement().getParseNode();
-        JXPathContext jxpContext = JXPathContextFactory.newInstance().newContext(null, root);
-
-        jxpContext.setLenient(true);
-
-        Iterator itr = jxpContext.iterate(xpath);
+        Iterator itr = JXPathUtils.newSafeContext(root, true).iterate(xpath);
 
         while (itr.hasNext()) {
             Object obj = itr.next();

@@ -0,0 +1,64 @@
+/*
+ *    GeoTools - The Open Source Java GIS Toolkit
+ *    http://geotools.org
+ *
+ *    (C) 2024, Open Source Geospatial Foundation (OSGeo)
+ *
+ *    This library is free software; you can redistribute it and/or
+ *    modify it under the terms of the GNU Lesser General Public
+ *    License as published by the Free Software Foundation;
+ *    version 2.1 of the License.
+ *
+ *    This library is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *    Lesser General Public License for more details.
+ */
+package org.geotools.xsd.impl.jxpath;
+
+import java.util.Enumeration;
+import org.apache.commons.jxpath.FunctionLibrary;
+import org.apache.commons.jxpath.JXPathContext;
+import org.xml.sax.helpers.NamespaceSupport;
+
+/** Contains utility methods to create safe {@code JXPathContext} objects. */
+public class JXPathUtils {
+
+    /**
+     * Creates a {@code JXPathContext} that disables calling Java methods from XPath expressions.
+     *
+     * @param contextBean the root node object
+     * @param lenient whether the context is in lenient mode
+     * @return the context
+     */
+    public static JXPathContext newSafeContext(Object contextBean, boolean lenient) {
+        return newSafeContext(contextBean, lenient, null, true);
+    }
+
+    /**
+     * Creates a {@code JXPathContext} that disables calling Java methods from XPath expressions.
+     *
+     * @param contextBean the root node object
+     * @param lenient whether the context is in lenient mode
+     * @param ns the namespaces
+     * @param declared true to include all declared prefixes; false for all active prefixes
+     * @return the context
+     */
+    @SuppressWarnings("unchecked")
+    public static JXPathContext newSafeContext(
+            Object contextBean, boolean lenient, NamespaceSupport ns, boolean declared) {
+        JXPathContext context = JXPathContext.newContext(contextBean);
+        context.setLenient(lenient);
+        if (ns != null) {
+            Enumeration<String> prefixes = declared ? ns.getDeclaredPrefixes() : ns.getPrefixes();
+            while (prefixes.hasMoreElements()) {
+                String prefix = prefixes.nextElement();
+                String uri = ns.getURI(prefix);
+                context.registerNamespace(prefix, uri);
+            }
+        }
+        // Set empty function library to prevent calling functions
+        context.setFunctions(new FunctionLibrary());
+        return context;
+    }
+}

@@ -17,6 +17,7 @@
 package org.geotools.xsd;
 
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.io.ByteArrayInputStream;
 import javax.xml.namespace.QName;
@@ -39,4 +40,16 @@ public void testParseXXE() throws Exception {
         // StreamingParser returns null if the parsing fails
         assertNull(parser.parse());
     }
+
+    @Test
+    public void testParseWithJavaMethod() throws Exception {
+        ByteArrayInputStream in = new ByteArrayInputStream("<mails></mails>".getBytes());
+        StreamingParser parser =
+                new StreamingParser(new MLConfiguration(), in, "java.lang.Thread.sleep(30000)");
+        // StreamingParser returns null if the parsing fails
+        long start = System.currentTimeMillis();
+        assertNull(parser.parse());
+        long runtime = System.currentTimeMillis() - start;
+        assertTrue("java.lang.Thread.sleep(30000) was executed", runtime < 30000);
+    }
 }