From 7701da7bef54544fc124b1ce82e812f91d9dcf59 Mon Sep 17 00:00:00 2001 From: bhmohanr-techie Date: Thu, 13 Oct 2022 17:33:10 +0530 Subject: [PATCH 01/14] Fix CVE-2022-41852 Those using JXPath to interpret untrusted XPath expressions may be vulnerable to a remote code execution attack. All JXPathContext class functions processing a XPath string are vulnerable except compile() and compilePath() function. The XPath expression can be used by an attacker to load any Java class from the classpath resulting in code execution. 9.8 Critical 1.3 and earlier Remote Code Execution (RCE) - Issue reported here is, all functions in the class JXPathContext (except compile and compilePath) are vulnerable to a remote code execution attack. - An arbitrary code can be injected in the xpath values passed to these functions, and it allows triggering java classes that can exploit the target machine. - For instance, the iterate() method in the JXPathContext class, can be invoked by passing the xpath argument value as, java.lang.Thread.sleep(9999999) or java.lang.Class.forName("ExploitTest"). These examples can result in triggering the injected java code, and can exploit the target machine. - Example: JXPathContext context = JXPathContext.newContext(new Test() ); Iterator result = context.iterate("java.lang.Thread.sleep(9999999)"); System.out.println("result.hasNext() - " + result.hasNext()); - No workaround available The fix added here is via a new system property "jxpath.class.allow". This list is empty by default meaning that nothing is allowed by default. Users should then explicitly configure which classes are allowed. - In order to fix this issue, a filter class "JXPathFilter.java" is added in JXPath. This filter object will be used to validate if the xpath passed to JXPath is a valid one - This new filter class, for now implements JXPathClassFilter interface, which is used to check the java classes passed in xpath. - In this filter, the class filter validates to see if the class being loaded in JXPath is part of the restriction list. The restriction can be configured via a system property "jxpath.class.allow" - System property "jxpath.class.allow" can be set to specify the list of allowed classnames. This property takes a list of java classnames (use comma as separator to specify more than one class). If this property is not set, it disallows all classes. - When a new extension function is created by JXPath, it uses the filter to check if the xpath supplied is a valid string, only then function instances are created. - The changes are added to pass JXPathFilter instance as an additional argument to the methods that create the ExtensionFunction objects. In addition, the ClassLoaderUtil class is modified to use the JXPathFilter instance when a new Class instance is created. Please note that, the changes here are added as overloaded methods, so that we are not affecting existing code for anyone. --- .../apache/commons/jxpath/ClassFunctions.java | 23 +++ .../commons/jxpath/FunctionLibrary.java | 27 +++- .../org/apache/commons/jxpath/Functions.java | 13 ++ .../commons/jxpath/PackageFunctions.java | 33 +++- .../commons/jxpath/ri/JXPathClassFilter.java | 41 +++++ .../jxpath/ri/JXPathContextReferenceImpl.java | 19 +-- .../commons/jxpath/ri/JXPathFilter.java | 67 ++++++++ .../commons/jxpath/util/ClassLoaderUtil.java | 82 +++++++++- .../ri/compiler/ExtensionFunctionTest.java | 149 ++++++++++++++++-- .../jxpath/ri/compiler/TestFunctions3.java | 55 +++++++ .../jxpath/ri/model/BeanModelTestCase.java | 16 +- .../jxpath/util/ClassLoaderUtilTest.java | 3 + 12 files changed, 482 insertions(+), 46 deletions(-) create mode 100644 src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java create mode 100644 src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java create mode 100644 src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java diff --git a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java index 9e77b595f..d63c39b22 100644 --- a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java @@ -23,6 +23,7 @@ import org.apache.commons.jxpath.functions.ConstructorFunction; import org.apache.commons.jxpath.functions.MethodFunction; +import org.apache.commons.jxpath.ri.JXPathFilter; import org.apache.commons.jxpath.util.MethodLookupUtils; /** @@ -91,6 +92,28 @@ public Function getFunction( final String namespace, final String name, Object[] parameters) { + return getFunction(namespace, name, parameters, null); + } + + public Function getFunction( + String namespace, + String name, + Object[] parameters, + JXPathFilter jxPathFilter) { + + // give chance to ClassFilter to filter out, if present + try { + if (jxPathFilter != null && !jxPathFilter.exposeToXPath(functionClass.getName())) { + throw new ClassNotFoundException(functionClass.getName()); + } + } + catch (ClassNotFoundException ex) { + throw new JXPathException( + "Cannot invoke extension function " + + (namespace != null ? namespace + ":" + name : name), + ex); + } + if (namespace == null) { if (this.namespace != null) { return null; diff --git a/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java b/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java index dfd4e75c0..f10a1b6ea 100644 --- a/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java +++ b/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java @@ -16,6 +16,8 @@ */ package org.apache.commons.jxpath; +import org.apache.commons.jxpath.ri.JXPathFilter; + import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -76,12 +78,30 @@ public Set getUsedNamespaces() { @Override public Function getFunction(final String namespace, final String name, final Object[] parameters) { - final Object candidates = functionCache().get(namespace); + return getFunction(namespace, name, parameters, null); + } + + /** + * Returns a Function, if any, for the specified namespace, + * name and parameter types. + * @param namespace function namespace + * @param name function name + * @param parameters parameters + * @param jxPathFilter the XPath filter + * @return Function found + */ + public Function getFunction( + final String namespace, + final String name, + final Object[] parameters, + final JXPathFilter jxPathFilter) { + Object candidates = functionCache().get(namespace); if (candidates instanceof Functions) { return ((Functions) candidates).getFunction( namespace, name, - parameters); + parameters, + jxPathFilter); } if (candidates instanceof List) { final List list = (List) candidates; @@ -91,7 +111,8 @@ public Function getFunction(final String namespace, final String name, ((Functions) list.get(i)).getFunction( namespace, name, - parameters); + parameters, + jxPathFilter); if (function != null) { return function; } diff --git a/src/main/java/org/apache/commons/jxpath/Functions.java b/src/main/java/org/apache/commons/jxpath/Functions.java index d3788c5c0..bdad84c49 100644 --- a/src/main/java/org/apache/commons/jxpath/Functions.java +++ b/src/main/java/org/apache/commons/jxpath/Functions.java @@ -16,6 +16,8 @@ */ package org.apache.commons.jxpath; +import org.apache.commons.jxpath.ri.JXPathFilter; + import java.util.Set; /** @@ -43,4 +45,15 @@ public interface Functions { * @return Function */ Function getFunction(String namespace, String name, Object[] parameters); + + /** + * Returns a Function, if any, for the specified namespace, + * name and parameter types. + * @param namespace ns + * @param name function name + * @param parameters Object[] + * @param jxPathFilter the XPath filter + * @return Function + */ + Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter jxPathFilter); } diff --git a/src/main/java/org/apache/commons/jxpath/PackageFunctions.java b/src/main/java/org/apache/commons/jxpath/PackageFunctions.java index 3a2e4a1ee..a8c045624 100644 --- a/src/main/java/org/apache/commons/jxpath/PackageFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/PackageFunctions.java @@ -26,6 +26,7 @@ import org.apache.commons.jxpath.functions.ConstructorFunction; import org.apache.commons.jxpath.functions.MethodFunction; +import org.apache.commons.jxpath.ri.JXPathFilter; import org.apache.commons.jxpath.util.ClassLoaderUtil; import org.apache.commons.jxpath.util.MethodLookupUtils; import org.apache.commons.jxpath.util.TypeUtils; @@ -116,6 +117,36 @@ public Function getFunction( final String namespace, final String name, Object[] parameters) { + return getFunction(namespace, name, parameters, null); + } + + /** + * Returns a {@link Function}, if found, for the specified namespace, + * name and parameter types. + *

+ * @param namespace - if it is not the same as specified in the + * construction, this method returns null + * @param name - name of the method, which can one these forms: + *

+ * @param parameters Object[] of parameters + * @param jxPathFilter the XPath filter + * @return a MethodFunction, a ConstructorFunction or null if no function + * is found + */ + public Function getFunction( + final String namespace, + final String name, + Object[] parameters, + final JXPathFilter jxPathFilter) { if (!Objects.equals(this.namespace, namespace)) { return null; } @@ -185,7 +216,7 @@ public Function getFunction( Class functionClass; try { - functionClass = ClassLoaderUtil.getClass(className, true); + functionClass = ClassLoaderUtil.getClass(className, true, jxPathFilter); } catch (final ClassNotFoundException ex) { throw new JXPathException( diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java new file mode 100644 index 000000000..8bc583d6a --- /dev/null +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.jxpath.ri; + +/** + * Class filter (optional) to be used by JXPath. + * + * System property "jxpath.class.allow" can be set to specify the list of allowd classnames. + * This property takes a list of java classnames (use comma as separator to specify more than one class). + * If this property is not set, it exposes no java classes + * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other + * classes will be not exposed. + * + * @author bhmohanr-techie + * @version $Revision$ $Date$ + */ +public interface JXPathClassFilter { + + /** + * Should the Java class of the specified name be exposed via xpath? + * @param className is the fully qualified name of the java class being + * checked. This will not be null. Only non-array class names will be + * passed. + * @return true if the java class can be exposed via xpath, false otherwise + */ + public boolean exposeToXPath(String className); +} diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java index 2d7319944..f36a6520a 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java @@ -27,23 +27,10 @@ import java.util.Map.Entry; import java.util.Vector; -import org.apache.commons.jxpath.CompiledExpression; -import org.apache.commons.jxpath.ExceptionHandler; -import org.apache.commons.jxpath.Function; -import org.apache.commons.jxpath.Functions; -import org.apache.commons.jxpath.JXPathContext; -import org.apache.commons.jxpath.JXPathException; -import org.apache.commons.jxpath.JXPathFunctionNotFoundException; -import org.apache.commons.jxpath.JXPathInvalidSyntaxException; -import org.apache.commons.jxpath.JXPathNotFoundException; -import org.apache.commons.jxpath.JXPathTypeConversionException; -import org.apache.commons.jxpath.Pointer; +import org.apache.commons.jxpath.*; import org.apache.commons.jxpath.ri.axes.InitialContext; import org.apache.commons.jxpath.ri.axes.RootContext; -import org.apache.commons.jxpath.ri.compiler.Expression; -import org.apache.commons.jxpath.ri.compiler.LocationPath; -import org.apache.commons.jxpath.ri.compiler.Path; -import org.apache.commons.jxpath.ri.compiler.TreeCompiler; +import org.apache.commons.jxpath.ri.compiler.*; import org.apache.commons.jxpath.ri.model.NodePointer; import org.apache.commons.jxpath.ri.model.NodePointerFactory; import org.apache.commons.jxpath.ri.model.VariablePointerFactory; @@ -765,7 +752,7 @@ public Function getFunction(final QName functionName, final Object[] parameters) while (funcCtx != null) { funcs = funcCtx.getFunctions(); if (funcs != null) { - func = funcs.getFunction(namespace, name, parameters); + func = funcs.getFunction(namespace, name, parameters, new JXPathFilter()); if (func != null) { return func; } diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java new file mode 100644 index 000000000..0e4d338ba --- /dev/null +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.jxpath.ri; + +import java.util.ArrayList; +import java.util.Arrays; + +/** + * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions. + * This class implements specific filter interfaces, and implements methods in those. + * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath + * JXPath uses this filter instance when an extension function instance is created. + * + * @author bhmohanr-techie + * @version $Revision$ $Date$ + */ +public class JXPathFilter implements JXPathClassFilter { + ArrayList allowedClassesList = null; + + public JXPathFilter() { + init(); + } + + public void init() { + String restrictedClasses = System.getProperty("jxpath.class.allow"); + allowedClassesList = null; + if ((restrictedClasses != null) && (restrictedClasses.trim().length() > 0)) { + allowedClassesList = new ArrayList(); + allowedClassesList.addAll(Arrays.asList(restrictedClasses.split(","))); + } + } + + /** + * Specifies whether the Java class of the specified name be exposed via xpath + * + * @param className is the fully qualified name of the java class being checked. + * This will not be null. Only non-array class names will be passed. + * @return true if the java class can be exposed via xpath, false otherwise + */ + @Override + public boolean exposeToXPath(String className) { + if ((allowedClassesList == null) || (allowedClassesList.size() < 1)) { + return false; + } + + if (allowedClassesList.contains(className) || allowedClassesList.contains("*")) { + return true; + } + + return false; + } +} + diff --git a/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java b/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java index fa1db5e66..acc34c0dc 100644 --- a/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java +++ b/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java @@ -16,6 +16,8 @@ */ package org.apache.commons.jxpath.util; +import org.apache.commons.jxpath.ri.JXPathFilter; + import java.util.HashMap; import java.util.Map; @@ -68,12 +70,19 @@ private static void addAbbreviation(final String primitive, final String abbrevi * @param classLoader the class loader to use to load the class * @param className the class name * @param initialize whether the class must be initialized + * @param jxPathFilter the XPath filter * @return the class represented by className using the classLoader * @throws ClassNotFoundException if the class is not found */ - public static Class getClass(final ClassLoader classLoader, final String className, final boolean initialize) + public static Class getClass(final ClassLoader classLoader, final String className, final boolean initialize, final JXPathFilter jxPathFilter) throws ClassNotFoundException { Class clazz; + + // give chance to ClassFilter to filter out, if present + if (jxPathFilter != null && !jxPathFilter.exposeToXPath(className)) { + throw new ClassNotFoundException(className); + } + if (abbreviationMap.containsKey(className)) { final String clsName = "[" + abbreviationMap.get(className); clazz = Class.forName(clsName, initialize, classLoader).getComponentType(); @@ -84,6 +93,38 @@ public static Class getClass(final ClassLoader classLoader, final String classNa return clazz; } + /** + * Returns the class represented by className using the + * classLoader. This implementation supports names like + * "java.lang.String[]" as well as "[Ljava.lang.String;". + * + * @param classLoader the class loader to use to load the class + * @param className the class name + * @param initialize whether the class must be initialized + * @return the class represented by className using the classLoader + * @throws ClassNotFoundException if the class is not found + */ + public static Class getClass(ClassLoader classLoader, String className, boolean initialize) + throws ClassNotFoundException { + return getClass(classLoader, className, initialize, null); + } + + /** + * Returns the (initialized) class represented by className + * using the classLoader. This implementation supports names + * like "java.lang.String[]" as well as + * "[Ljava.lang.String;". + * + * @param classLoader the class loader to use to load the class + * @param className the class name + * @param jxPathFilter the XPath filter + * @return the class represented by className using the classLoader + * @throws ClassNotFoundException if the class is not found + */ + public static Class getClass(ClassLoader classLoader, String className, JXPathFilter jxPathFilter) throws ClassNotFoundException { + return getClass(classLoader, className, true, jxPathFilter); + } + /** * Returns the (initialized) class represented by className * using the classLoader. This implementation supports names @@ -96,7 +137,7 @@ public static Class getClass(final ClassLoader classLoader, final String classNa * @throws ClassNotFoundException if the class is not found */ public static Class getClass(final ClassLoader classLoader, final String className) throws ClassNotFoundException { - return getClass(classLoader, className, true); + return getClass(classLoader, className, true, null); } /** @@ -110,7 +151,22 @@ public static Class getClass(final ClassLoader classLoader, final String classNa * @throws ClassNotFoundException if the class is not found */ public static Class getClass(final String className) throws ClassNotFoundException { - return getClass(className, true); + return getClass(className, true, null); + } + + /** + * Returns the (initialized) class represented by className + * using the current thread's context class loader. This implementation + * supports names like "java.lang.String[]" as well as + * "[Ljava.lang.String;". + * + * @param className the class name + * @param jxPathFilter the XPath filter + * @return the class represented by className using the current thread's context class loader + * @throws ClassNotFoundException if the class is not found + */ + public static Class getClass(String className, JXPathFilter jxPathFilter) throws ClassNotFoundException { + return getClass(className, true, jxPathFilter); } /** @@ -125,17 +181,33 @@ public static Class getClass(final String className) throws ClassNotFoundExcepti * @throws ClassNotFoundException if the class is not found */ public static Class getClass(final String className, final boolean initialize) throws ClassNotFoundException { + return getClass(className, initialize, null); + } + + /** + * Returns the class represented by className using the + * current thread's context class loader. This implementation supports + * names like "java.lang.String[]" as well as + * "[Ljava.lang.String;". + * + * @param className the class name + * @param initialize whether the class must be initialized + * @param jxPathFilter the XPath filter + * @return the class represented by className using the current thread's context class loader + * @throws ClassNotFoundException if the class is not found + */ + public static Class getClass(final String className, final boolean initialize, final JXPathFilter jxPathFilter) throws ClassNotFoundException { final ClassLoader contextCL = Thread.currentThread().getContextClassLoader(); final ClassLoader currentCL = ClassLoaderUtil.class.getClassLoader(); if (contextCL != null) { try { - return getClass(contextCL, className, initialize); + return getClass(contextCL, className, initialize, jxPathFilter); } catch (final ClassNotFoundException ignore) { // NOPMD // ignore this exception and try the current class loader } } - return getClass(currentCL, className, initialize); + return getClass(currentCL, className, initialize, jxPathFilter); } /** diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java index 7ab105e9f..8bd5fa1a0 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java @@ -16,23 +16,10 @@ */ package org.apache.commons.jxpath.ri.compiler; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Locale; - -import org.apache.commons.jxpath.ClassFunctions; -import org.apache.commons.jxpath.ExpressionContext; -import org.apache.commons.jxpath.Function; -import org.apache.commons.jxpath.FunctionLibrary; -import org.apache.commons.jxpath.Functions; -import org.apache.commons.jxpath.JXPathContext; -import org.apache.commons.jxpath.JXPathTestCase; -import org.apache.commons.jxpath.NodeSet; -import org.apache.commons.jxpath.PackageFunctions; -import org.apache.commons.jxpath.Pointer; -import org.apache.commons.jxpath.TestBean; -import org.apache.commons.jxpath.Variables; +import java.util.*; + +import org.apache.commons.jxpath.*; +import org.apache.commons.jxpath.ri.JXPathFilter; import org.apache.commons.jxpath.ri.model.NodePointer; import org.apache.commons.jxpath.util.JXPath11CompatibleTypeConverter; import org.apache.commons.jxpath.util.TypeConverter; @@ -46,9 +33,12 @@ public class ExtensionFunctionTest extends JXPathTestCase { private JXPathContext context; private TestBean testBean; private TypeConverter typeConverter; + JXPathFilter jxPathFilter = new JXPathFilter(); @Override public void setUp() { + System.setProperty("jxpath.class.allow", + "org.apache.commons.jxpath.ri.compiler.TestFunctions,org.apache.commons.jxpath.ri.compiler.TestFunctions2"); if (context == null) { testBean = new TestBean(); context = JXPathContext.newContext(testBean); @@ -75,6 +65,7 @@ public void setUp() { @Override public void tearDown() { TypeUtils.setTypeConverter(typeConverter); + System.clearProperty("jxpath.class.allow"); } public void testConstructorLookup() { @@ -384,6 +375,130 @@ public void testBCNodeSetHack() { Boolean.TRUE); } + public void testClassFunctionsWithoutClassFilter() { + Function classFunction = null; + try { + Functions iFunctions = new ClassFunctions(TestFunctions3.class, "test3"); + + jxPathFilter.init(); + + classFunction = iFunctions.getFunction("test3", "testFunction3Method1", null, jxPathFilter); + } catch (Throwable t) { + assertTrue((t.getMessage().contains("Cannot invoke extension function test3:testFunction3Method1; org.apache.commons.jxpath.ri.compiler.TestFunctions3")) + || (t.getMessage().contains("java.lang.ClassNotFoundException: org.apache.commons.jxpath.ri.compiler.TestFunctions3"))); + } finally { + assertNull(classFunction); + } + } + + public void testClassFunctionsWithClassFilter() { + Function classFunction = null; + try { + Functions iFunctions = new ClassFunctions(TestFunctions2.class, "test"); + + jxPathFilter.init(); + + classFunction = iFunctions.getFunction("test", "increment", new Object[]{8}, jxPathFilter); + } catch (Throwable t) { + fail(t.getMessage()); + } finally { + assertNotNull(classFunction); + } + } + + public void testPackageFunctionsWithoutClassFilter() { + Function packageFunction = null; + try { + Functions iFunctions = new PackageFunctions("org.apache.commons.jxpath.ri.compiler.","jxpathtests"); + + jxPathFilter.init(); + + packageFunction = iFunctions.getFunction("jxpathtests", "TestFunctions3.testFunction3Method1", null, jxPathFilter); + throw new Exception("testPackageFunctionsWithClassFilter() failed."); + } catch (Throwable t) { + assertTrue((t.getMessage().contains("Cannot invoke extension function jxpathtests:TestFunctions3.testFunction3Method1; org.apache.commons.jxpath.ri.compiler.TestFunctions3")) + || (t.getMessage().contains("java.lang.ClassNotFoundException: org.apache.commons.jxpath.ri.compiler.TestFunctions3"))); + } finally { + assertNull(packageFunction); + } + } + + public void testPackageFunctionsWithClassFilter() { + Function packageFunction = null; + String defaultAllowList = System.getProperty("jxpath.class.allow"); + try { + Functions iFunctions = new PackageFunctions("org.apache.commons.jxpath.ri.compiler.","jxpathtests"); + System.setProperty("jxpath.class.allow", defaultAllowList + ",org.apache.commons.jxpath.ri.compiler.TestFunctions3"); + jxPathFilter.init(); + + packageFunction = iFunctions.getFunction("jxpathtests", "TestFunctions3.testFunction3Method1", null, jxPathFilter); + } catch (Throwable t) { + fail(t.getMessage()); + } finally { + assertNotNull(packageFunction); + System.setProperty("jxpath.class.allow", defaultAllowList); + } + } + + public void testJXPathContextFunctionsWithoutClassFilter() { + String failedMethods = null; + try { + jxPathFilter.init(); + + try { + context.iterate("java.lang.Thread.sleep(5000)"); + throw new Exception("testJXPathContextFunctionsWithClassFilter() failed for iterate()"); + } catch (Throwable t) { + if ((t.getMessage().indexOf("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread") > -1) + || (t.getMessage().indexOf("java.lang.ClassNotFoundException: java.lang.Thread") > -1)) { + //success + } else { + failedMethods = "org.apache.commons.jxpath.JXPathContext.iterate()"; + } + } + + try { + context.selectSingleNode("java.lang.Thread.sleep(5000)"); + throw new Exception("testJXPathContextFunctionsWithClassFilter() failed for iterate()"); + } catch (Throwable t) { + if ((t.getMessage().indexOf("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread") > -1) + || (t.getMessage().indexOf("java.lang.ClassNotFoundException: java.lang.Thread") > -1)) { + //success + } else { + failedMethods += ("".equals(failedMethods) ? "org.apache.commons.jxpath.JXPathContext.selectSingleNode()" : ", org.apache.commons.jxpath.JXPathContext.selectSingleNode()"); + } + } + + } catch (Throwable t) { + fail(t.getMessage()); + } finally { + if (failedMethods != null) { + fail("Problem exists in: " + failedMethods); + } + } + } + + public void testJXPathContextFunctionsWithClassFilter() { + try { + System.setProperty("jxpath.class.allow", "java.lang.Thread"); + jxPathFilter.init(); + + long t = System.currentTimeMillis(); + context.iterate("java.lang.Thread.sleep(5000)"); + t = System.currentTimeMillis() - t; + assertTrue(t >= 5); + + t = System.currentTimeMillis(); + context.selectSingleNode("java.lang.Thread.sleep(5000)"); + t = System.currentTimeMillis() - t; + assertTrue(t >= 5); + } catch (Throwable t) { + fail(t.getMessage()); + } finally { + System.clearProperty("jxpath.class.allow"); + } + } + private static class Context implements ExpressionContext { private final Object object; diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java new file mode 100644 index 000000000..d8d0c92a6 --- /dev/null +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.jxpath.ri.compiler; + +import org.apache.commons.jxpath.Functions; +import org.apache.commons.jxpath.JXPathContext; + +import java.util.*; + +/** + * A test class with few methods, with different argument list + * + * @author bhmohanr-techie + * @version $Revision$ $Date$ + */ +public final class TestFunctions3 { + + static { + System.out.println("TestFunctions3: static block..."); + } + + public TestFunctions3() { + System.out.println("TestFunctions3: constructor..."); + } + + public static String testFunction3Method1() { + System.out.println("TestFunctions3: testFunction3Method1 method..."); + return "testFunction3Method1"; + } + + public String testFunction3Method2(String str) { + System.out.println("TestFunctions3: testFunction3Method2 method..." + str); + return "testFunction3Method2:" + str; + } + + public String testFunction3Method3(String str1, String str2) { + System.out.println("TestFunctions3: testFunction3Method3 method..." + str1 + ", " + str2); + return "testFunction3Method3:" + str1 + ":" + str2; + } + +} \ No newline at end of file diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/BeanModelTestCase.java b/src/test/java/org/apache/commons/jxpath/ri/model/BeanModelTestCase.java index fa9ee56b2..c8a948555 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/model/BeanModelTestCase.java +++ b/src/test/java/org/apache/commons/jxpath/ri/model/BeanModelTestCase.java @@ -42,14 +42,22 @@ public abstract class BeanModelTestCase extends JXPathTestCase { private JXPathContext context; @Override - public void setUp() { + public void setUp() throws Exception { // if (context == null) { - context = JXPathContext.newContext(createContextBean()); - context.setLocale(Locale.US); - context.setFactory(getAbstractFactory()); + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + context = JXPathContext.newContext(createContextBean()); + context.setLocale(Locale.US); + context.setFactory(getAbstractFactory()); // } } + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + protected abstract Object createContextBean(); protected abstract AbstractFactory getAbstractFactory(); diff --git a/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java b/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java index 729f989db..2ab26724f 100644 --- a/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java +++ b/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java @@ -117,6 +117,7 @@ public static void callExampleMessageMethodAndAssertClassNotFoundJXPathException * JXPath and asserts the dynamic class load succeeds. */ public static void callExampleMessageMethodAndAssertSuccess() { + System.setProperty("jxpath.class.allow", EXAMPLE_CLASS_NAME); final JXPathContext context = JXPathContext.newContext(new Object()); Object value; try { @@ -124,6 +125,8 @@ public static void callExampleMessageMethodAndAssertSuccess() { assertEquals("an example class", value); } catch( final Exception e ) { fail(e.getMessage()); + } finally { + System.clearProperty("jxpath.class.allow"); } } From 02dc4f50425640b65ece4553e60ddc5b084082d3 Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 05:42:17 +0200 Subject: [PATCH 02/14] Throw JXPathException directly when filtering a function class --- .../org/apache/commons/jxpath/ClassFunctions.java | 14 ++++---------- .../jxpath/ri/compiler/ExtensionFunctionTest.java | 4 ++-- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java index d63c39b22..139e359ba 100644 --- a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java @@ -102,17 +102,11 @@ public Function getFunction( JXPathFilter jxPathFilter) { // give chance to ClassFilter to filter out, if present - try { - if (jxPathFilter != null && !jxPathFilter.exposeToXPath(functionClass.getName())) { - throw new ClassNotFoundException(functionClass.getName()); - } - } - catch (ClassNotFoundException ex) { + if (jxPathFilter != null && !jxPathFilter.exposeToXPath(functionClass.getName())) { throw new JXPathException( - "Cannot invoke extension function " - + (namespace != null ? namespace + ":" + name : name), - ex); - } + "Extension function is not allowed: " + (namespace != null ? namespace + ":" + name : name) + + " (in " + functionClass.getName() + ")"); + } if (namespace == null) { if (this.namespace != null) { diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java index 8bd5fa1a0..7aeb21b8d 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java @@ -384,8 +384,8 @@ public void testClassFunctionsWithoutClassFilter() { classFunction = iFunctions.getFunction("test3", "testFunction3Method1", null, jxPathFilter); } catch (Throwable t) { - assertTrue((t.getMessage().contains("Cannot invoke extension function test3:testFunction3Method1; org.apache.commons.jxpath.ri.compiler.TestFunctions3")) - || (t.getMessage().contains("java.lang.ClassNotFoundException: org.apache.commons.jxpath.ri.compiler.TestFunctions3"))); + System.err.println(t.getMessage()); + assertTrue((t.getMessage().contains("Extension function is not allowed: test3:testFunction3Method1 (in org.apache.commons.jxpath.ri.compiler.TestFunctions3)"))); } finally { assertNull(classFunction); } From c248c0b9c0fce946bca263a2f7c758cc389b589d Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 06:41:53 +0200 Subject: [PATCH 03/14] Enable the allow-list by default for all getFunction methods --- src/main/java/org/apache/commons/jxpath/ClassFunctions.java | 2 +- src/main/java/org/apache/commons/jxpath/FunctionLibrary.java | 2 +- src/main/java/org/apache/commons/jxpath/PackageFunctions.java | 2 +- src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java index 139e359ba..ed0c13c3b 100644 --- a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java @@ -92,7 +92,7 @@ public Function getFunction( final String namespace, final String name, Object[] parameters) { - return getFunction(namespace, name, parameters, null); + return getFunction(namespace, name, parameters, new JXPathFilter()); } public Function getFunction( diff --git a/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java b/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java index f10a1b6ea..740623b43 100644 --- a/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java +++ b/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java @@ -78,7 +78,7 @@ public Set getUsedNamespaces() { @Override public Function getFunction(final String namespace, final String name, final Object[] parameters) { - return getFunction(namespace, name, parameters, null); + return getFunction(namespace, name, parameters, new JXPathFilter()); } /** diff --git a/src/main/java/org/apache/commons/jxpath/PackageFunctions.java b/src/main/java/org/apache/commons/jxpath/PackageFunctions.java index a8c045624..c7c43efdf 100644 --- a/src/main/java/org/apache/commons/jxpath/PackageFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/PackageFunctions.java @@ -117,7 +117,7 @@ public Function getFunction( final String namespace, final String name, Object[] parameters) { - return getFunction(namespace, name, parameters, null); + return getFunction(namespace, name, parameters, new JXPathFilter()); } /** diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java index 0e4d338ba..18b0a1f29 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java @@ -39,7 +39,7 @@ public void init() { String restrictedClasses = System.getProperty("jxpath.class.allow"); allowedClassesList = null; if ((restrictedClasses != null) && (restrictedClasses.trim().length() > 0)) { - allowedClassesList = new ArrayList(); + allowedClassesList = new ArrayList<>(); allowedClassesList.addAll(Arrays.asList(restrictedClasses.split(","))); } } From 6d5e13f29f3916fb3742e1915486f100707aefed Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 06:42:41 +0200 Subject: [PATCH 04/14] Update JXPathClassFilter documentation to mention the allow-all option --- .../java/org/apache/commons/jxpath/ri/JXPathClassFilter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java index 8bc583d6a..d5a37f3b1 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java @@ -19,11 +19,11 @@ /** * Class filter (optional) to be used by JXPath. * - * System property "jxpath.class.allow" can be set to specify the list of allowd classnames. + * System property "jxpath.class.allow" can be set to specify the list of allowed classnames. * This property takes a list of java classnames (use comma as separator to specify more than one class). * If this property is not set, it exposes no java classes * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other - * classes will be not exposed. + * classes will be not exposed. You can use the wildcard (*) to allow all classes. * * @author bhmohanr-techie * @version $Revision$ $Date$ From 0e337f0bdaaa3780a59957827f2111bd444d6ece Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 06:42:56 +0200 Subject: [PATCH 05/14] Organize Imports --- .../jxpath/ri/JXPathContextReferenceImpl.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java index f36a6520a..4a8e156e4 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java @@ -27,10 +27,23 @@ import java.util.Map.Entry; import java.util.Vector; -import org.apache.commons.jxpath.*; +import org.apache.commons.jxpath.CompiledExpression; +import org.apache.commons.jxpath.ExceptionHandler; +import org.apache.commons.jxpath.Function; +import org.apache.commons.jxpath.Functions; +import org.apache.commons.jxpath.JXPathContext; +import org.apache.commons.jxpath.JXPathException; +import org.apache.commons.jxpath.JXPathFunctionNotFoundException; +import org.apache.commons.jxpath.JXPathInvalidSyntaxException; +import org.apache.commons.jxpath.JXPathNotFoundException; +import org.apache.commons.jxpath.JXPathTypeConversionException; +import org.apache.commons.jxpath.Pointer; import org.apache.commons.jxpath.ri.axes.InitialContext; import org.apache.commons.jxpath.ri.axes.RootContext; -import org.apache.commons.jxpath.ri.compiler.*; +import org.apache.commons.jxpath.ri.compiler.Expression; +import org.apache.commons.jxpath.ri.compiler.LocationPath; +import org.apache.commons.jxpath.ri.compiler.Path; +import org.apache.commons.jxpath.ri.compiler.TreeCompiler; import org.apache.commons.jxpath.ri.model.NodePointer; import org.apache.commons.jxpath.ri.model.NodePointerFactory; import org.apache.commons.jxpath.ri.model.VariablePointerFactory; From b30628943784a9545c7ec3c6067885e604694bac Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 06:45:50 +0200 Subject: [PATCH 06/14] Convert the restricted classes into a Set --- .../java/org/apache/commons/jxpath/ri/JXPathFilter.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java index 18b0a1f29..3192f85b8 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java @@ -16,8 +16,9 @@ */ package org.apache.commons.jxpath.ri; -import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; /** * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions. @@ -29,7 +30,7 @@ * @version $Revision$ $Date$ */ public class JXPathFilter implements JXPathClassFilter { - ArrayList allowedClassesList = null; + Set allowedClassesList = null; public JXPathFilter() { init(); @@ -39,7 +40,7 @@ public void init() { String restrictedClasses = System.getProperty("jxpath.class.allow"); allowedClassesList = null; if ((restrictedClasses != null) && (restrictedClasses.trim().length() > 0)) { - allowedClassesList = new ArrayList<>(); + allowedClassesList = new HashSet<>(); allowedClassesList.addAll(Arrays.asList(restrictedClasses.split(","))); } } From ff5ab0a6bbfdfe3951436dc8ac55a9aeedcaae08 Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 06:56:00 +0200 Subject: [PATCH 07/14] Fix Checkstyle and PMD errors --- .../apache/commons/jxpath/ri/JXPathClassFilter.java | 2 +- .../org/apache/commons/jxpath/ri/JXPathFilter.java | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java index d5a37f3b1..1367bf40b 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java @@ -37,5 +37,5 @@ public interface JXPathClassFilter { * passed. * @return true if the java class can be exposed via xpath, false otherwise */ - public boolean exposeToXPath(String className); + boolean exposeToXPath(String className); } diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java index 3192f85b8..bba8d900e 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java @@ -30,7 +30,7 @@ * @version $Revision$ $Date$ */ public class JXPathFilter implements JXPathClassFilter { - Set allowedClassesList = null; + private Set allowedClassesList = null; public JXPathFilter() { init(); @@ -39,7 +39,7 @@ public JXPathFilter() { public void init() { String restrictedClasses = System.getProperty("jxpath.class.allow"); allowedClassesList = null; - if ((restrictedClasses != null) && (restrictedClasses.trim().length() > 0)) { + if (restrictedClasses != null && restrictedClasses.trim().length() > 0) { allowedClassesList = new HashSet<>(); allowedClassesList.addAll(Arrays.asList(restrictedClasses.split(","))); } @@ -54,15 +54,11 @@ public void init() { */ @Override public boolean exposeToXPath(String className) { - if ((allowedClassesList == null) || (allowedClassesList.size() < 1)) { + if (allowedClassesList == null || allowedClassesList.size() < 1) { return false; } - if (allowedClassesList.contains(className) || allowedClassesList.contains("*")) { - return true; - } - - return false; + return allowedClassesList.contains(className) || allowedClassesList.contains("*"); } } From 021b8cef1ec0d6d253e3481bb7c2411dff43e42d Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 17:30:25 +0200 Subject: [PATCH 08/14] Minor refactoring and clean-up --- .../apache/commons/jxpath/ri/JXPathClassFilter.java | 4 +--- .../org/apache/commons/jxpath/ri/JXPathFilter.java | 11 +++-------- .../commons/jxpath/ri/compiler/TestFunctions3.java | 10 +--------- 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java index 1367bf40b..5f01483d9 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java @@ -24,9 +24,7 @@ * If this property is not set, it exposes no java classes * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other * classes will be not exposed. You can use the wildcard (*) to allow all classes. - * - * @author bhmohanr-techie - * @version $Revision$ $Date$ + * @since 1.4 */ public interface JXPathClassFilter { diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java index bba8d900e..92554ed9b 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java @@ -25,9 +25,6 @@ * This class implements specific filter interfaces, and implements methods in those. * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath * JXPath uses this filter instance when an extension function instance is created. - * - * @author bhmohanr-techie - * @version $Revision$ $Date$ */ public class JXPathFilter implements JXPathClassFilter { private Set allowedClassesList = null; @@ -37,11 +34,10 @@ public JXPathFilter() { } public void init() { - String restrictedClasses = System.getProperty("jxpath.class.allow"); - allowedClassesList = null; - if (restrictedClasses != null && restrictedClasses.trim().length() > 0) { + final String allowedClasses = System.getProperty("jxpath.class.allow"); + if (allowedClasses != null && !allowedClasses.isEmpty()) { allowedClassesList = new HashSet<>(); - allowedClassesList.addAll(Arrays.asList(restrictedClasses.split(","))); + allowedClassesList.addAll(Arrays.asList(allowedClasses.split(","))); } } @@ -61,4 +57,3 @@ public boolean exposeToXPath(String className) { return allowedClassesList.contains(className) || allowedClassesList.contains("*"); } } - diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java index d8d0c92a6..0205041c8 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java @@ -16,16 +16,8 @@ */ package org.apache.commons.jxpath.ri.compiler; -import org.apache.commons.jxpath.Functions; -import org.apache.commons.jxpath.JXPathContext; - -import java.util.*; - /** * A test class with few methods, with different argument list - * - * @author bhmohanr-techie - * @version $Revision$ $Date$ */ public final class TestFunctions3 { @@ -52,4 +44,4 @@ public String testFunction3Method3(String str1, String str2) { return "testFunction3Method3:" + str1 + ":" + str2; } -} \ No newline at end of file +} From 27d1ec0c5b014618d98e3f481d4260ceb833babf Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 21:47:18 +0200 Subject: [PATCH 09/14] Use JXPathFilter based on the `jxpath.allow.class` system property This has the additional benefit of not breaking the binary compatibility of the Functions interface --- .../apache/commons/jxpath/ClassFunctions.java | 13 +-- .../commons/jxpath/FunctionLibrary.java | 27 +---- .../org/apache/commons/jxpath/Functions.java | 13 --- .../commons/jxpath/PackageFunctions.java | 33 +----- .../jxpath/ri/JXPathContextReferenceImpl.java | 2 +- .../commons/jxpath/util/ClassLoaderUtil.java | 6 +- .../commons/jxpath/BasicNodeSetTest.java | 7 ++ .../commons/jxpath/issues/JXPath118Test.java | 11 ++ .../commons/jxpath/issues/JXPath149Test.java | 12 ++ .../jxpath/issues/JXPath172DynamicTest.java | 12 ++ .../ri/compiler/ExtensionFunctionTest.java | 107 ++++++++---------- .../model/AliasedNamespaceIterationTest.java | 16 ++- .../commons/jxpath/ri/model/XMLSpaceTest.java | 12 ++ .../ri/model/XMLUpperCaseElementsTest.java | 12 ++ .../servlet/JXPathServletContextTest.java | 12 ++ .../jxpath/util/ClassLoaderUtilTest.java | 6 +- 16 files changed, 152 insertions(+), 149 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java index ed0c13c3b..aeb51f771 100644 --- a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java @@ -92,17 +92,8 @@ public Function getFunction( final String namespace, final String name, Object[] parameters) { - return getFunction(namespace, name, parameters, new JXPathFilter()); - } - - public Function getFunction( - String namespace, - String name, - Object[] parameters, - JXPathFilter jxPathFilter) { - - // give chance to ClassFilter to filter out, if present - if (jxPathFilter != null && !jxPathFilter.exposeToXPath(functionClass.getName())) { + JXPathFilter jxPathFilter = new JXPathFilter(); + if (!jxPathFilter.exposeToXPath(functionClass.getName())) { throw new JXPathException( "Extension function is not allowed: " + (namespace != null ? namespace + ":" + name : name) + " (in " + functionClass.getName() + ")"); diff --git a/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java b/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java index 740623b43..dfd4e75c0 100644 --- a/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java +++ b/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java @@ -16,8 +16,6 @@ */ package org.apache.commons.jxpath; -import org.apache.commons.jxpath.ri.JXPathFilter; - import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -78,30 +76,12 @@ public Set getUsedNamespaces() { @Override public Function getFunction(final String namespace, final String name, final Object[] parameters) { - return getFunction(namespace, name, parameters, new JXPathFilter()); - } - - /** - * Returns a Function, if any, for the specified namespace, - * name and parameter types. - * @param namespace function namespace - * @param name function name - * @param parameters parameters - * @param jxPathFilter the XPath filter - * @return Function found - */ - public Function getFunction( - final String namespace, - final String name, - final Object[] parameters, - final JXPathFilter jxPathFilter) { - Object candidates = functionCache().get(namespace); + final Object candidates = functionCache().get(namespace); if (candidates instanceof Functions) { return ((Functions) candidates).getFunction( namespace, name, - parameters, - jxPathFilter); + parameters); } if (candidates instanceof List) { final List list = (List) candidates; @@ -111,8 +91,7 @@ public Function getFunction( ((Functions) list.get(i)).getFunction( namespace, name, - parameters, - jxPathFilter); + parameters); if (function != null) { return function; } diff --git a/src/main/java/org/apache/commons/jxpath/Functions.java b/src/main/java/org/apache/commons/jxpath/Functions.java index bdad84c49..d3788c5c0 100644 --- a/src/main/java/org/apache/commons/jxpath/Functions.java +++ b/src/main/java/org/apache/commons/jxpath/Functions.java @@ -16,8 +16,6 @@ */ package org.apache.commons.jxpath; -import org.apache.commons.jxpath.ri.JXPathFilter; - import java.util.Set; /** @@ -45,15 +43,4 @@ public interface Functions { * @return Function */ Function getFunction(String namespace, String name, Object[] parameters); - - /** - * Returns a Function, if any, for the specified namespace, - * name and parameter types. - * @param namespace ns - * @param name function name - * @param parameters Object[] - * @param jxPathFilter the XPath filter - * @return Function - */ - Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter jxPathFilter); } diff --git a/src/main/java/org/apache/commons/jxpath/PackageFunctions.java b/src/main/java/org/apache/commons/jxpath/PackageFunctions.java index c7c43efdf..3a2e4a1ee 100644 --- a/src/main/java/org/apache/commons/jxpath/PackageFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/PackageFunctions.java @@ -26,7 +26,6 @@ import org.apache.commons.jxpath.functions.ConstructorFunction; import org.apache.commons.jxpath.functions.MethodFunction; -import org.apache.commons.jxpath.ri.JXPathFilter; import org.apache.commons.jxpath.util.ClassLoaderUtil; import org.apache.commons.jxpath.util.MethodLookupUtils; import org.apache.commons.jxpath.util.TypeUtils; @@ -117,36 +116,6 @@ public Function getFunction( final String namespace, final String name, Object[] parameters) { - return getFunction(namespace, name, parameters, new JXPathFilter()); - } - - /** - * Returns a {@link Function}, if found, for the specified namespace, - * name and parameter types. - *

- * @param namespace - if it is not the same as specified in the - * construction, this method returns null - * @param name - name of the method, which can one these forms: - *

    - *
  • methodname, if invoking a method on an object passed as the - * first parameter
    • - *
  • Classname.new, if looking for a constructor
    • - *
  • subpackage.subpackage.Classname.new, if looking for a - * constructor in a subpackage
    • - *
  • Classname.methodname, if looking for a static method
    • - *
  • subpackage.subpackage.Classname.methodname, if looking for a - * static method of a class in a subpackage
    • - *
- * @param parameters Object[] of parameters - * @param jxPathFilter the XPath filter - * @return a MethodFunction, a ConstructorFunction or null if no function - * is found - */ - public Function getFunction( - final String namespace, - final String name, - Object[] parameters, - final JXPathFilter jxPathFilter) { if (!Objects.equals(this.namespace, namespace)) { return null; } @@ -216,7 +185,7 @@ public Function getFunction( Class functionClass; try { - functionClass = ClassLoaderUtil.getClass(className, true, jxPathFilter); + functionClass = ClassLoaderUtil.getClass(className, true); } catch (final ClassNotFoundException ex) { throw new JXPathException( diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java index 4a8e156e4..2d7319944 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java @@ -765,7 +765,7 @@ public Function getFunction(final QName functionName, final Object[] parameters) while (funcCtx != null) { funcs = funcCtx.getFunctions(); if (funcs != null) { - func = funcs.getFunction(namespace, name, parameters, new JXPathFilter()); + func = funcs.getFunction(namespace, name, parameters); if (func != null) { return func; } diff --git a/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java b/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java index acc34c0dc..4a8597389 100644 --- a/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java +++ b/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java @@ -79,7 +79,7 @@ public static Class getClass(final ClassLoader classLoader, final String classNa Class clazz; // give chance to ClassFilter to filter out, if present - if (jxPathFilter != null && !jxPathFilter.exposeToXPath(className)) { + if (jxPathFilter == null || !jxPathFilter.exposeToXPath(className)) { throw new ClassNotFoundException(className); } @@ -106,7 +106,7 @@ public static Class getClass(final ClassLoader classLoader, final String classNa */ public static Class getClass(ClassLoader classLoader, String className, boolean initialize) throws ClassNotFoundException { - return getClass(classLoader, className, initialize, null); + return getClass(classLoader, className, initialize, new JXPathFilter()); } /** @@ -181,7 +181,7 @@ public static Class getClass(String className, JXPathFilter jxPathFilter) throws * @throws ClassNotFoundException if the class is not found */ public static Class getClass(final String className, final boolean initialize) throws ClassNotFoundException { - return getClass(className, initialize, null); + return getClass(className, initialize, new JXPathFilter()); } /** diff --git a/src/test/java/org/apache/commons/jxpath/BasicNodeSetTest.java b/src/test/java/org/apache/commons/jxpath/BasicNodeSetTest.java index 26643511e..d4066b1f3 100644 --- a/src/test/java/org/apache/commons/jxpath/BasicNodeSetTest.java +++ b/src/test/java/org/apache/commons/jxpath/BasicNodeSetTest.java @@ -35,10 +35,17 @@ public class BasicNodeSetTest extends JXPathTestCase { @Override protected void setUp() throws Exception { super.setUp(); + System.setProperty("jxpath.class.allow", "*"); context = JXPathContext.newContext(new TestMixedModelBean()); nodeSet = new BasicNodeSet(); } + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + /** * Add the pointers for the specified path to nodeSet. * diff --git a/src/test/java/org/apache/commons/jxpath/issues/JXPath118Test.java b/src/test/java/org/apache/commons/jxpath/issues/JXPath118Test.java index 93c9c3fd8..4138a61d5 100644 --- a/src/test/java/org/apache/commons/jxpath/issues/JXPath118Test.java +++ b/src/test/java/org/apache/commons/jxpath/issues/JXPath118Test.java @@ -29,6 +29,17 @@ */ public class JXPath118Test extends TestCase { + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } public void testJXPATH118IssueWithAsPath() throws Exception { diff --git a/src/test/java/org/apache/commons/jxpath/issues/JXPath149Test.java b/src/test/java/org/apache/commons/jxpath/issues/JXPath149Test.java index dc4558f7d..b2d4d0bc0 100644 --- a/src/test/java/org/apache/commons/jxpath/issues/JXPath149Test.java +++ b/src/test/java/org/apache/commons/jxpath/issues/JXPath149Test.java @@ -21,6 +21,18 @@ public class JXPath149Test extends JXPathTestCase { + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + public void testComplexOperationWithVariables() { final JXPathContext context = JXPathContext.newContext(null); context.getVariables().declareVariable("a", Integer.valueOf(0)); diff --git a/src/test/java/org/apache/commons/jxpath/issues/JXPath172DynamicTest.java b/src/test/java/org/apache/commons/jxpath/issues/JXPath172DynamicTest.java index 6899ab753..282ec2403 100644 --- a/src/test/java/org/apache/commons/jxpath/issues/JXPath172DynamicTest.java +++ b/src/test/java/org/apache/commons/jxpath/issues/JXPath172DynamicTest.java @@ -36,6 +36,18 @@ public static TestSuite suite() return new TestSuite(JXPath172DynamicTest.class); } + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + public void testIssue172_propertyExistAndIsNotNull() { final JXPathContext context = getContext("ciao", false); diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java index 7aeb21b8d..6b92889f9 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java @@ -16,10 +16,23 @@ */ package org.apache.commons.jxpath.ri.compiler; -import java.util.*; - -import org.apache.commons.jxpath.*; -import org.apache.commons.jxpath.ri.JXPathFilter; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Locale; + +import org.apache.commons.jxpath.JXPathTestCase; +import org.apache.commons.jxpath.Functions; +import org.apache.commons.jxpath.JXPathContext; +import org.apache.commons.jxpath.TestBean; +import org.apache.commons.jxpath.Variables; +import org.apache.commons.jxpath.FunctionLibrary; +import org.apache.commons.jxpath.ClassFunctions; +import org.apache.commons.jxpath.PackageFunctions; +import org.apache.commons.jxpath.NodeSet; +import org.apache.commons.jxpath.Function; +import org.apache.commons.jxpath.ExpressionContext; +import org.apache.commons.jxpath.Pointer; import org.apache.commons.jxpath.ri.model.NodePointer; import org.apache.commons.jxpath.util.JXPath11CompatibleTypeConverter; import org.apache.commons.jxpath.util.TypeConverter; @@ -33,7 +46,6 @@ public class ExtensionFunctionTest extends JXPathTestCase { private JXPathContext context; private TestBean testBean; private TypeConverter typeConverter; - JXPathFilter jxPathFilter = new JXPathFilter(); @Override public void setUp() { @@ -379,10 +391,7 @@ public void testClassFunctionsWithoutClassFilter() { Function classFunction = null; try { Functions iFunctions = new ClassFunctions(TestFunctions3.class, "test3"); - - jxPathFilter.init(); - - classFunction = iFunctions.getFunction("test3", "testFunction3Method1", null, jxPathFilter); + classFunction = iFunctions.getFunction("test3", "testFunction3Method1", null); } catch (Throwable t) { System.err.println(t.getMessage()); assertTrue((t.getMessage().contains("Extension function is not allowed: test3:testFunction3Method1 (in org.apache.commons.jxpath.ri.compiler.TestFunctions3)"))); @@ -395,10 +404,7 @@ public void testClassFunctionsWithClassFilter() { Function classFunction = null; try { Functions iFunctions = new ClassFunctions(TestFunctions2.class, "test"); - - jxPathFilter.init(); - - classFunction = iFunctions.getFunction("test", "increment", new Object[]{8}, jxPathFilter); + classFunction = iFunctions.getFunction("test", "increment", new Object[]{8}); } catch (Throwable t) { fail(t.getMessage()); } finally { @@ -409,11 +415,8 @@ public void testClassFunctionsWithClassFilter() { public void testPackageFunctionsWithoutClassFilter() { Function packageFunction = null; try { - Functions iFunctions = new PackageFunctions("org.apache.commons.jxpath.ri.compiler.","jxpathtests"); - - jxPathFilter.init(); - - packageFunction = iFunctions.getFunction("jxpathtests", "TestFunctions3.testFunction3Method1", null, jxPathFilter); + Functions iFunctions = new PackageFunctions("org.apache.commons.jxpath.ri.compiler.", "jxpathtests"); + packageFunction = iFunctions.getFunction("jxpathtests", "TestFunctions3.testFunction3Method1", null); throw new Exception("testPackageFunctionsWithClassFilter() failed."); } catch (Throwable t) { assertTrue((t.getMessage().contains("Cannot invoke extension function jxpathtests:TestFunctions3.testFunction3Method1; org.apache.commons.jxpath.ri.compiler.TestFunctions3")) @@ -427,11 +430,9 @@ public void testPackageFunctionsWithClassFilter() { Function packageFunction = null; String defaultAllowList = System.getProperty("jxpath.class.allow"); try { - Functions iFunctions = new PackageFunctions("org.apache.commons.jxpath.ri.compiler.","jxpathtests"); + Functions iFunctions = new PackageFunctions("org.apache.commons.jxpath.ri.compiler.", "jxpathtests"); System.setProperty("jxpath.class.allow", defaultAllowList + ",org.apache.commons.jxpath.ri.compiler.TestFunctions3"); - jxPathFilter.init(); - - packageFunction = iFunctions.getFunction("jxpathtests", "TestFunctions3.testFunction3Method1", null, jxPathFilter); + packageFunction = iFunctions.getFunction("jxpathtests", "TestFunctions3.testFunction3Method1", null); } catch (Throwable t) { fail(t.getMessage()); } finally { @@ -441,57 +442,41 @@ public void testPackageFunctionsWithClassFilter() { } public void testJXPathContextFunctionsWithoutClassFilter() { - String failedMethods = null; + String failedMethods = ""; try { - jxPathFilter.init(); - - try { - context.iterate("java.lang.Thread.sleep(5000)"); - throw new Exception("testJXPathContextFunctionsWithClassFilter() failed for iterate()"); - } catch (Throwable t) { - if ((t.getMessage().indexOf("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread") > -1) - || (t.getMessage().indexOf("java.lang.ClassNotFoundException: java.lang.Thread") > -1)) { - //success - } else { - failedMethods = "org.apache.commons.jxpath.JXPathContext.iterate()"; - } - } - - try { - context.selectSingleNode("java.lang.Thread.sleep(5000)"); - throw new Exception("testJXPathContextFunctionsWithClassFilter() failed for iterate()"); - } catch (Throwable t) { - if ((t.getMessage().indexOf("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread") > -1) - || (t.getMessage().indexOf("java.lang.ClassNotFoundException: java.lang.Thread") > -1)) { - //success - } else { - failedMethods += ("".equals(failedMethods) ? "org.apache.commons.jxpath.JXPathContext.selectSingleNode()" : ", org.apache.commons.jxpath.JXPathContext.selectSingleNode()"); - } + context.iterate("java.lang.Thread.sleep(5000)"); + throw new Exception("testJXPathContextFunctionsWithClassFilter() failed for iterate()"); + } catch (Throwable t) { + if (!t.getMessage().contains("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread") + && !t.getMessage().contains("java.lang.ClassNotFoundException: java.lang.Thread")) { + failedMethods = "org.apache.commons.jxpath.JXPathContext.iterate()"; } + } + try { + context.selectSingleNode("java.lang.Thread.sleep(5000)"); + throw new Exception("testJXPathContextFunctionsWithClassFilter() failed for iterate()"); } catch (Throwable t) { - fail(t.getMessage()); - } finally { - if (failedMethods != null) { - fail("Problem exists in: " + failedMethods); + if (!(t.getMessage().contains("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread")) + && !(t.getMessage().contains("java.lang.ClassNotFoundException: java.lang.Thread"))) { + failedMethods += ("".equals(failedMethods) ? "org.apache.commons.jxpath.JXPathContext.selectSingleNode()" : ", org.apache.commons.jxpath.JXPathContext.selectSingleNode()"); } } + if (!failedMethods.isEmpty()) { + fail("Failed filtering for methods: " + failedMethods); + } } public void testJXPathContextFunctionsWithClassFilter() { try { System.setProperty("jxpath.class.allow", "java.lang.Thread"); - jxPathFilter.init(); + long startTime = System.currentTimeMillis(); + context.iterate("java.lang.Thread.sleep(5)"); + assertTrue(System.currentTimeMillis() >= startTime + 5); - long t = System.currentTimeMillis(); - context.iterate("java.lang.Thread.sleep(5000)"); - t = System.currentTimeMillis() - t; - assertTrue(t >= 5); - - t = System.currentTimeMillis(); - context.selectSingleNode("java.lang.Thread.sleep(5000)"); - t = System.currentTimeMillis() - t; - assertTrue(t >= 5); + startTime = System.currentTimeMillis(); + context.selectSingleNode("java.lang.Thread.sleep(50)"); + assertTrue(System.currentTimeMillis() >= startTime + 5); } catch (Throwable t) { fail(t.getMessage()); } finally { diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/AliasedNamespaceIterationTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/AliasedNamespaceIterationTest.java index 4c97e2bbd..db693ac02 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/model/AliasedNamespaceIterationTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/model/AliasedNamespaceIterationTest.java @@ -24,14 +24,24 @@ /** * Test aliased/doubled XML namespace iteration; JXPATH-125. - * */ public class AliasedNamespaceIterationTest extends JXPathTestCase { protected JXPathContext context; + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + protected DocumentContainer createDocumentContainer(final String model) { - final DocumentContainer result = new DocumentContainer(JXPathTestCase.class - .getResource("IterateAliasedNS.xml"), model); + final DocumentContainer result = new DocumentContainer(JXPathTestCase.class.getResource("IterateAliasedNS.xml"), model); return result; } diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/XMLSpaceTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/XMLSpaceTest.java index bdbe0ab4e..e0f826386 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/model/XMLSpaceTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/model/XMLSpaceTest.java @@ -26,6 +26,18 @@ public class XMLSpaceTest extends JXPathTestCase { protected JXPathContext context; + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + protected DocumentContainer createDocumentContainer(final String model) { return new DocumentContainer(JXPathTestCase.class .getResource("XmlSpace.xml"), model); diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/XMLUpperCaseElementsTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/XMLUpperCaseElementsTest.java index afa65c487..cfe11b7af 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/model/XMLUpperCaseElementsTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/model/XMLUpperCaseElementsTest.java @@ -27,6 +27,18 @@ public class XMLUpperCaseElementsTest extends JXPathTestCase { protected JXPathContext context; + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + protected DocumentContainer createDocumentContainer(final String model) { return new DocumentContainer(JXPathTestCase.class.getResource("VendorUpper.xml"), model); } diff --git a/src/test/java/org/apache/commons/jxpath/servlet/JXPathServletContextTest.java b/src/test/java/org/apache/commons/jxpath/servlet/JXPathServletContextTest.java index 0b8159240..028fd95c7 100644 --- a/src/test/java/org/apache/commons/jxpath/servlet/JXPathServletContextTest.java +++ b/src/test/java/org/apache/commons/jxpath/servlet/JXPathServletContextTest.java @@ -38,6 +38,18 @@ */ public class JXPathServletContextTest extends TestCase { + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + private ServletContext getServletContext() { final MockServletContext context = new MockServletContext(); context.setAttribute("app", "OK"); diff --git a/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java b/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java index 2ab26724f..8e53857e2 100644 --- a/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java +++ b/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java @@ -39,6 +39,8 @@ public class ClassLoaderUtilTest extends TestCase { private static final String TEST_CASE_CLASS_NAME = "org.apache.commons.jxpath.util.ClassLoaderUtilTest"; private static final String EXAMPLE_CLASS_NAME = "org.apache.commons.jxpath.util.ClassLoadingExampleClass"; + private static final String DEFAULT_FACTORY_CLASS = "org.apache.commons.jxpath.ri.JXPathContextFactoryReferenceImpl"; + private ClassLoader orginalContextClassLoader; /** @@ -46,6 +48,7 @@ public class ClassLoaderUtilTest extends TestCase { */ @Override public void setUp() { + System.setProperty("jxpath.class.allow", "*"); this.orginalContextClassLoader = Thread.currentThread().getContextClassLoader(); } @@ -55,6 +58,7 @@ public void setUp() { @Override public void tearDown() { Thread.currentThread().setContextClassLoader(this.orginalContextClassLoader); + System.clearProperty("jxpath.class.allow"); } /** @@ -117,7 +121,7 @@ public static void callExampleMessageMethodAndAssertClassNotFoundJXPathException * JXPath and asserts the dynamic class load succeeds. */ public static void callExampleMessageMethodAndAssertSuccess() { - System.setProperty("jxpath.class.allow", EXAMPLE_CLASS_NAME); + System.setProperty("jxpath.class.allow", EXAMPLE_CLASS_NAME+","+DEFAULT_FACTORY_CLASS); final JXPathContext context = JXPathContext.newContext(new Object()); Object value; try { From abb96a5fcb4736219beeb84a5b8cfa2a10663f2f Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Wed, 19 Oct 2022 22:05:57 +0200 Subject: [PATCH 10/14] Refactor the JXPathFilter Now JXPathFilter is an interface that should be used by any class which needs the filtering functionality. A default implementation based on the system property is provided by the SystemPropertyJXPathFilter class. The functions interface is enhanced with a default function that takes a filter parameter. The choice to have a default function is to not break binary compatibility. --- .../apache/commons/jxpath/ClassFunctions.java | 13 +++- .../org/apache/commons/jxpath/Functions.java | 15 ++++ .../commons/jxpath/ri/JXPathClassFilter.java | 39 ----------- .../commons/jxpath/ri/JXPathFilter.java | 48 ++++--------- .../jxpath/ri/SystemPropertyJXPathFilter.java | 68 +++++++++++++++++++ .../commons/jxpath/util/ClassLoaderUtil.java | 11 +-- .../ri/compiler/ExtensionFunctionTest.java | 7 +- .../jxpath/ri/model/JXPath154Test.java | 12 ++++ .../jxpath/ri/model/MixedModelTest.java | 7 ++ 9 files changed, 136 insertions(+), 84 deletions(-) delete mode 100644 src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java create mode 100644 src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java diff --git a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java index aeb51f771..6e09419cb 100644 --- a/src/main/java/org/apache/commons/jxpath/ClassFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/ClassFunctions.java @@ -24,6 +24,7 @@ import org.apache.commons.jxpath.functions.ConstructorFunction; import org.apache.commons.jxpath.functions.MethodFunction; import org.apache.commons.jxpath.ri.JXPathFilter; +import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; import org.apache.commons.jxpath.util.MethodLookupUtils; /** @@ -92,8 +93,16 @@ public Function getFunction( final String namespace, final String name, Object[] parameters) { - JXPathFilter jxPathFilter = new JXPathFilter(); - if (!jxPathFilter.exposeToXPath(functionClass.getName())) { + return getFunction(namespace, name, parameters, new SystemPropertyJXPathFilter()); + } + + @Override + public Function getFunction( + final String namespace, + final String name, + Object[] parameters, + JXPathFilter jxPathFilter) { + if (!jxPathFilter.isClassNameExposed(functionClass.getName())) { throw new JXPathException( "Extension function is not allowed: " + (namespace != null ? namespace + ":" + name : name) + " (in " + functionClass.getName() + ")"); diff --git a/src/main/java/org/apache/commons/jxpath/Functions.java b/src/main/java/org/apache/commons/jxpath/Functions.java index d3788c5c0..bd6d005b9 100644 --- a/src/main/java/org/apache/commons/jxpath/Functions.java +++ b/src/main/java/org/apache/commons/jxpath/Functions.java @@ -16,6 +16,8 @@ */ package org.apache.commons.jxpath; +import org.apache.commons.jxpath.ri.JXPathFilter; + import java.util.Set; /** @@ -43,4 +45,17 @@ public interface Functions { * @return Function */ Function getFunction(String namespace, String name, Object[] parameters); + + /** + * Returns a Function, if any, for the specified namespace, + * name and parameter types if the function is allowed by the filter. + * @param namespace ns + * @param name function name + * @param parameters Object[] + * @param filter JXPathFilter + * @return Function + */ + default Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter filter) { + return getFunction(namespace, name, parameters); + } } diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java deleted file mode 100644 index 5f01483d9..000000000 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.commons.jxpath.ri; - -/** - * Class filter (optional) to be used by JXPath. - * - * System property "jxpath.class.allow" can be set to specify the list of allowed classnames. - * This property takes a list of java classnames (use comma as separator to specify more than one class). - * If this property is not set, it exposes no java classes - * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other - * classes will be not exposed. You can use the wildcard (*) to allow all classes. - * @since 1.4 - */ -public interface JXPathClassFilter { - - /** - * Should the Java class of the specified name be exposed via xpath? - * @param className is the fully qualified name of the java class being - * checked. This will not be null. Only non-array class names will be - * passed. - * @return true if the java class can be exposed via xpath, false otherwise - */ - boolean exposeToXPath(String className); -} diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java index 92554ed9b..d66fe2c3d 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java @@ -16,44 +16,24 @@ */ package org.apache.commons.jxpath.ri; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; - /** - * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions. - * This class implements specific filter interfaces, and implements methods in those. - * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath - * JXPath uses this filter instance when an extension function instance is created. + * Class filter (optional) to be used by JXPath. + * + * System property "jxpath.class.allow" can be set to specify the list of allowed classnames. + * This property takes a list of java classnames (use comma as separator to specify more than one class). + * If this property is not set, it exposes no java classes + * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other + * classes will be not exposed. You can use the wildcard (*) to allow all classes. + * @since 1.4 */ -public class JXPathFilter implements JXPathClassFilter { - private Set allowedClassesList = null; - - public JXPathFilter() { - init(); - } - - public void init() { - final String allowedClasses = System.getProperty("jxpath.class.allow"); - if (allowedClasses != null && !allowedClasses.isEmpty()) { - allowedClassesList = new HashSet<>(); - allowedClassesList.addAll(Arrays.asList(allowedClasses.split(","))); - } - } +public interface JXPathFilter { /** - * Specifies whether the Java class of the specified name be exposed via xpath - * - * @param className is the fully qualified name of the java class being checked. - * This will not be null. Only non-array class names will be passed. + * Should the Java class of the specified name be exposed via xpath? + * @param className is the fully qualified name of the java class being + * checked. This will not be null. Only non-array class names will be + * passed. * @return true if the java class can be exposed via xpath, false otherwise */ - @Override - public boolean exposeToXPath(String className) { - if (allowedClassesList == null || allowedClassesList.size() < 1) { - return false; - } - - return allowedClassesList.contains(className) || allowedClassesList.contains("*"); - } + boolean isClassNameExposed(String className); } diff --git a/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java new file mode 100644 index 000000000..8393f8070 --- /dev/null +++ b/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.jxpath.ri; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Objects; +import java.util.Set; + +/** + * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions. + * This class implements specific filter interfaces, and implements methods in those. + * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath + * JXPath uses this filter instance when an extension function instance is created. + */ +public class SystemPropertyJXPathFilter implements JXPathFilter { + private final Set allowedClasses; + + public SystemPropertyJXPathFilter() { + this.allowedClasses = new HashSet<>(); + final String allowedClasses = System.getProperty("jxpath.class.allow"); + if (allowedClasses != null && !allowedClasses.isEmpty()) { + this.allowedClasses.addAll(Arrays.asList(allowedClasses.split(","))); + } + } + + /** + * Specifies whether the Java class of the specified name be exposed via xpath + * + * @param className is the fully qualified name of the java class being checked. + * This will not be null. Only non-array class names will be passed. + * @return true if the java class can be exposed via xpath, false otherwise + */ + @Override + public boolean isClassNameExposed(String className) { + if (allowedClasses.isEmpty()) { + return false; + } + + if (allowedClasses.contains("*")) { + return true; + } + + return allowedClasses.stream().anyMatch(pattern -> { + if (Objects.equals(className, pattern)) { + return true; + } + else if (pattern.endsWith("*")) { + return className.startsWith(pattern.substring(0, pattern.length() - 1)); + } + return false; + }); + } +} diff --git a/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java b/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java index 4a8597389..0f0281da7 100644 --- a/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java +++ b/src/main/java/org/apache/commons/jxpath/util/ClassLoaderUtil.java @@ -17,6 +17,7 @@ package org.apache.commons.jxpath.util; import org.apache.commons.jxpath.ri.JXPathFilter; +import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; import java.util.HashMap; import java.util.Map; @@ -79,7 +80,7 @@ public static Class getClass(final ClassLoader classLoader, final String classNa Class clazz; // give chance to ClassFilter to filter out, if present - if (jxPathFilter == null || !jxPathFilter.exposeToXPath(className)) { + if (jxPathFilter == null || !jxPathFilter.isClassNameExposed(className)) { throw new ClassNotFoundException(className); } @@ -106,7 +107,7 @@ public static Class getClass(final ClassLoader classLoader, final String classNa */ public static Class getClass(ClassLoader classLoader, String className, boolean initialize) throws ClassNotFoundException { - return getClass(classLoader, className, initialize, new JXPathFilter()); + return getClass(classLoader, className, initialize, new SystemPropertyJXPathFilter()); } /** @@ -137,7 +138,7 @@ public static Class getClass(ClassLoader classLoader, String className, JXPathFi * @throws ClassNotFoundException if the class is not found */ public static Class getClass(final ClassLoader classLoader, final String className) throws ClassNotFoundException { - return getClass(classLoader, className, true, null); + return getClass(classLoader, className, true, new SystemPropertyJXPathFilter()); } /** @@ -151,7 +152,7 @@ public static Class getClass(final ClassLoader classLoader, final String classNa * @throws ClassNotFoundException if the class is not found */ public static Class getClass(final String className) throws ClassNotFoundException { - return getClass(className, true, null); + return getClass(className, true, new SystemPropertyJXPathFilter()); } /** @@ -181,7 +182,7 @@ public static Class getClass(String className, JXPathFilter jxPathFilter) throws * @throws ClassNotFoundException if the class is not found */ public static Class getClass(final String className, final boolean initialize) throws ClassNotFoundException { - return getClass(className, initialize, new JXPathFilter()); + return getClass(className, initialize, new SystemPropertyJXPathFilter()); } /** diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java index 6b92889f9..d3cd117ad 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java @@ -50,7 +50,7 @@ public class ExtensionFunctionTest extends JXPathTestCase { @Override public void setUp() { System.setProperty("jxpath.class.allow", - "org.apache.commons.jxpath.ri.compiler.TestFunctions,org.apache.commons.jxpath.ri.compiler.TestFunctions2"); + "java.lang.String,java.util.*,org.apache.commons.jxpath.ri.compiler.TestFunctions,org.apache.commons.jxpath.ri.compiler.TestFunctions2,org.apache.commons.jxpath.ri.JXPath*"); if (context == null) { testBean = new TestBean(); context = JXPathContext.newContext(testBean); @@ -393,7 +393,6 @@ public void testClassFunctionsWithoutClassFilter() { Functions iFunctions = new ClassFunctions(TestFunctions3.class, "test3"); classFunction = iFunctions.getFunction("test3", "testFunction3Method1", null); } catch (Throwable t) { - System.err.println(t.getMessage()); assertTrue((t.getMessage().contains("Extension function is not allowed: test3:testFunction3Method1 (in org.apache.commons.jxpath.ri.compiler.TestFunctions3)"))); } finally { assertNull(classFunction); @@ -444,7 +443,7 @@ public void testPackageFunctionsWithClassFilter() { public void testJXPathContextFunctionsWithoutClassFilter() { String failedMethods = ""; try { - context.iterate("java.lang.Thread.sleep(5000)"); + context.iterate("java.lang.Thread.sleep(5)"); throw new Exception("testJXPathContextFunctionsWithClassFilter() failed for iterate()"); } catch (Throwable t) { if (!t.getMessage().contains("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread") @@ -454,7 +453,7 @@ public void testJXPathContextFunctionsWithoutClassFilter() { } try { - context.selectSingleNode("java.lang.Thread.sleep(5000)"); + context.selectSingleNode("java.lang.Thread.sleep(5)"); throw new Exception("testJXPathContextFunctionsWithClassFilter() failed for iterate()"); } catch (Throwable t) { if (!(t.getMessage().contains("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread")) diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/JXPath154Test.java b/src/test/java/org/apache/commons/jxpath/ri/model/JXPath154Test.java index eff6253eb..7352365c9 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/model/JXPath154Test.java +++ b/src/test/java/org/apache/commons/jxpath/ri/model/JXPath154Test.java @@ -25,6 +25,18 @@ public class JXPath154Test extends JXPathTestCase { protected JXPathContext context; + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } + protected DocumentContainer createDocumentContainer(final String model) { return new DocumentContainer(JXPathTestCase.class.getResource("InnerEmptyNamespace.xml"), model); } diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java index e4b23d53f..ceb30e758 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java @@ -41,6 +41,7 @@ public class MixedModelTest extends JXPathTestCase { @Override public void setUp() { + System.setProperty("jxpath.class.allow", "java.util.*"); final TestMixedModelBean bean = new TestMixedModelBean(); context = JXPathContext.newContext(bean); context.setFactory(new TestMixedModelFactory()); @@ -61,6 +62,12 @@ public void setUp() { vars.declareVariable("matrix", matrix); } + @Override + protected void tearDown() throws Exception { + super.tearDown(); + System.clearProperty("jxpath.class.allow"); + } + public void testVar() { context.getVariables().declareVariable("foo:bar", "baz"); From 36bc1b2ffa22c257fc6d37662a9ff0865f235d6c Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Fri, 21 Oct 2022 07:27:03 +0200 Subject: [PATCH 11/14] Add filtering to the MethodFunction class --- .../jxpath/functions/MethodFunction.java | 8 ++++ .../JXPathContextReferenceImplTestCase.java | 28 +++++++++++++- .../commons/jxpath/ri/TestDangerousClass.java | 37 +++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/apache/commons/jxpath/ri/TestDangerousClass.java diff --git a/src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java b/src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java index 28dd87d31..2b8aaefb0 100644 --- a/src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java +++ b/src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java @@ -23,6 +23,8 @@ import org.apache.commons.jxpath.ExpressionContext; import org.apache.commons.jxpath.Function; import org.apache.commons.jxpath.JXPathInvalidAccessException; +import org.apache.commons.jxpath.ri.JXPathFilter; +import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; import org.apache.commons.jxpath.util.TypeUtils; import org.apache.commons.jxpath.util.ValueUtils; @@ -34,6 +36,8 @@ public class MethodFunction implements Function { private final Method method; private static final Object[] EMPTY_ARRAY = {}; + private JXPathFilter jxpathFilter = new SystemPropertyJXPathFilter(); + /** * Create a new MethodFunction. * @param method implementing Method @@ -88,7 +92,11 @@ public Object invoke(final ExpressionContext context, Object[] parameters) { } } + if (!jxpathFilter.isClassNameExposed(method.getDeclaringClass().getName())) { + throw new Exception("Calling method is not allowed: " + method.getDeclaringClass() + "." + method.getName() + "()"); + } return method.invoke(target, args); + } catch (Throwable ex) { if (ex instanceof InvocationTargetException) { diff --git a/src/test/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImplTestCase.java b/src/test/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImplTestCase.java index 369ccdf80..17f83c49c 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImplTestCase.java +++ b/src/test/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImplTestCase.java @@ -17,11 +17,24 @@ package org.apache.commons.jxpath.ri; +import org.apache.commons.jxpath.JXPathTestCase; import org.apache.commons.jxpath.ri.model.container.ContainerPointerFactory; +import org.apache.commons.jxpath.JXPathContext; -import junit.framework.TestCase; -public class JXPathContextReferenceImplTestCase extends TestCase { +public class JXPathContextReferenceImplTestCase extends JXPathTestCase { + + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "org.apache.commons.jxpath.ri.JXPathContextFactoryReferenceImpl"); + } + + @Override + protected void tearDown() throws Exception { + super.tearDown(); + System.clearProperty("jxpath.class.allow"); + } /** * https://issues.apache.org/jira/browse/JXPATH-166 @@ -36,4 +49,15 @@ public void testInit() { } } } + + public void testDangerousClass() { + try { + JXPathContext context = JXPathContext.newContext(new Object() {}); + String jxPath = "run(newInstance(loadClass(getClassLoader(getClass(/)), \"org.apache.commons.jxpath.ri.TestDangerousClass\")), \"blabla\")"; + context.getValue(jxPath); + fail("failed to block org.apache.commons.jxpath.ri.TestDangerousClass.run()"); + } catch (Exception e) { + assertTrue(e.getMessage().contains("Calling method is not allowed: class java.lang.Object.getClass()")); + } + } } diff --git a/src/test/java/org/apache/commons/jxpath/ri/TestDangerousClass.java b/src/test/java/org/apache/commons/jxpath/ri/TestDangerousClass.java new file mode 100644 index 000000000..93cb575aa --- /dev/null +++ b/src/test/java/org/apache/commons/jxpath/ri/TestDangerousClass.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.jxpath.ri; + +/** + * A test class with few methods, with different argument list + */ +public final class TestDangerousClass { + + static { + System.out.println("TestDangerousClass created: static block!"); + } + + public TestDangerousClass() { + System.out.println("TestDangerousClass: constructor!"); + } + + public void run(String param) { + System.out.printf("Running dangerous test with parameter '%s'!%n", param); +// System.exit(42); + } + +} From b0e0000fe5b4096f293a77db21ce54e0690e7579 Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Fri, 21 Oct 2022 09:21:06 +0200 Subject: [PATCH 12/14] Fix tests --- .../jxpath/ri/SystemPropertyJXPathFilter.java | 16 ++++++---------- .../commons/jxpath/issues/JXPath113Test.java | 11 +++++++++++ .../jxpath/ri/compiler/CoreFunctionTest.java | 7 +++++++ .../ri/compiler/ExtensionFunctionTest.java | 13 ++++++++----- .../commons/jxpath/ri/model/MixedModelTest.java | 2 +- .../jxpath/ri/model/XMLModelTestCase.java | 7 +++++++ .../commons/jxpath/util/ClassLoaderUtilTest.java | 7 +------ 7 files changed, 41 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java index 8393f8070..461931e55 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java @@ -16,10 +16,7 @@ */ package org.apache.commons.jxpath.ri; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Objects; -import java.util.Set; +import java.util.*; /** * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions. @@ -31,11 +28,11 @@ public class SystemPropertyJXPathFilter implements JXPathFilter { private final Set allowedClasses; public SystemPropertyJXPathFilter() { - this.allowedClasses = new HashSet<>(); final String allowedClasses = System.getProperty("jxpath.class.allow"); - if (allowedClasses != null && !allowedClasses.isEmpty()) { - this.allowedClasses.addAll(Arrays.asList(allowedClasses.split(","))); - } + List allowedClassesList = (allowedClasses != null && !allowedClasses.isEmpty()) + ? Arrays.asList(allowedClasses.split(",")) + : Collections.emptyList(); + this.allowedClasses = Collections.unmodifiableSet(new HashSet<>(allowedClassesList)); } /** @@ -58,8 +55,7 @@ public boolean isClassNameExposed(String className) { return allowedClasses.stream().anyMatch(pattern -> { if (Objects.equals(className, pattern)) { return true; - } - else if (pattern.endsWith("*")) { + } else if (pattern.endsWith("*")) { return className.startsWith(pattern.substring(0, pattern.length() - 1)); } return false; diff --git a/src/test/java/org/apache/commons/jxpath/issues/JXPath113Test.java b/src/test/java/org/apache/commons/jxpath/issues/JXPath113Test.java index 57f894c2f..073c939ec 100644 --- a/src/test/java/org/apache/commons/jxpath/issues/JXPath113Test.java +++ b/src/test/java/org/apache/commons/jxpath/issues/JXPath113Test.java @@ -29,6 +29,17 @@ public class JXPath113Test extends JXPathTestCase { + @Override + public void setUp() throws Exception { + super.setUp(); + System.setProperty("jxpath.class.allow", "*"); + } + + @Override + public void tearDown() throws Exception { + System.clearProperty("jxpath.class.allow"); + super.tearDown(); + } public void testIssue113() throws Exception { diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/CoreFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/CoreFunctionTest.java index 4610d2550..342f0092d 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/CoreFunctionTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/CoreFunctionTest.java @@ -39,6 +39,7 @@ public class CoreFunctionTest extends JXPathTestCase { @Override public void setUp() { + System.setProperty("jxpath.class.allow", "*"); if (context == null) { context = JXPathContext.newContext(new TestMixedModelBean()); final Variables vars = context.getVariables(); @@ -48,6 +49,12 @@ public void setUp() { } } + @Override + protected void tearDown() throws Exception { + super.tearDown(); + System.clearProperty("jxpath.class.allow"); + } + public void testCoreFunctions() { assertXPathValue(context, "string(2)", "2"); assertXPathValue(context, "string($nan)", "NaN"); diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java index d3cd117ad..fb4d13851 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java @@ -46,11 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase { private JXPathContext context; private TestBean testBean; private TypeConverter typeConverter; + private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*"; @Override public void setUp() { - System.setProperty("jxpath.class.allow", - "java.lang.String,java.util.*,org.apache.commons.jxpath.ri.compiler.TestFunctions,org.apache.commons.jxpath.ri.compiler.TestFunctions2,org.apache.commons.jxpath.ri.JXPath*"); + System.setProperty("jxpath.class.allow", DEFAULT_ALLOW_LIST); if (context == null) { testBean = new TestBean(); context = JXPathContext.newContext(testBean); @@ -388,6 +388,7 @@ public void testBCNodeSetHack() { } public void testClassFunctionsWithoutClassFilter() { + System.setProperty("jxpath.class.allow", "org.w3c.*,org.jdom.*,java.lang.String,java.util.*"); Function classFunction = null; try { Functions iFunctions = new ClassFunctions(TestFunctions3.class, "test3"); @@ -396,6 +397,7 @@ public void testClassFunctionsWithoutClassFilter() { assertTrue((t.getMessage().contains("Extension function is not allowed: test3:testFunction3Method1 (in org.apache.commons.jxpath.ri.compiler.TestFunctions3)"))); } finally { assertNull(classFunction); + System.setProperty("jxpath.class.allow", DEFAULT_ALLOW_LIST); } } @@ -412,6 +414,7 @@ public void testClassFunctionsWithClassFilter() { } public void testPackageFunctionsWithoutClassFilter() { + System.setProperty("jxpath.class.allow", "org.w3c.*,org.jdom.*,java.lang.String,java.util.*"); Function packageFunction = null; try { Functions iFunctions = new PackageFunctions("org.apache.commons.jxpath.ri.compiler.", "jxpathtests"); @@ -422,21 +425,21 @@ public void testPackageFunctionsWithoutClassFilter() { || (t.getMessage().contains("java.lang.ClassNotFoundException: org.apache.commons.jxpath.ri.compiler.TestFunctions3"))); } finally { assertNull(packageFunction); + System.setProperty("jxpath.class.allow", DEFAULT_ALLOW_LIST); } } public void testPackageFunctionsWithClassFilter() { + System.setProperty("jxpath.class.allow", "org.apache.commons.jxpath.ri.compiler.TestFunctions3"); Function packageFunction = null; - String defaultAllowList = System.getProperty("jxpath.class.allow"); try { Functions iFunctions = new PackageFunctions("org.apache.commons.jxpath.ri.compiler.", "jxpathtests"); - System.setProperty("jxpath.class.allow", defaultAllowList + ",org.apache.commons.jxpath.ri.compiler.TestFunctions3"); packageFunction = iFunctions.getFunction("jxpathtests", "TestFunctions3.testFunction3Method1", null); } catch (Throwable t) { fail(t.getMessage()); } finally { assertNotNull(packageFunction); - System.setProperty("jxpath.class.allow", defaultAllowList); + System.setProperty("jxpath.class.allow", DEFAULT_ALLOW_LIST); } } diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java index ceb30e758..0473c9c53 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java @@ -41,7 +41,7 @@ public class MixedModelTest extends JXPathTestCase { @Override public void setUp() { - System.setProperty("jxpath.class.allow", "java.util.*"); + System.setProperty("jxpath.class.allow", "java.util.*,org.apache.commons.jxpath.*"); final TestMixedModelBean bean = new TestMixedModelBean(); context = JXPathContext.newContext(bean); context.setFactory(new TestMixedModelFactory()); diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/XMLModelTestCase.java b/src/test/java/org/apache/commons/jxpath/ri/model/XMLModelTestCase.java index 4939cf9c3..2a81d7dc9 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/model/XMLModelTestCase.java +++ b/src/test/java/org/apache/commons/jxpath/ri/model/XMLModelTestCase.java @@ -36,6 +36,7 @@ public abstract class XMLModelTestCase extends JXPathTestCase { @Override public void setUp() { + System.setProperty("jxpath.class.allow", "*"); if (context == null) { final DocumentContainer docCtr = createDocumentContainer(); context = createContext(); @@ -46,6 +47,12 @@ public void setUp() { } } + @Override + protected void tearDown() throws Exception { + super.tearDown(); + System.clearProperty("jxpath.class.allow"); + } + protected abstract String getModel(); protected DocumentContainer createDocumentContainer() { diff --git a/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java b/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java index 8e53857e2..b622744a8 100644 --- a/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java +++ b/src/test/java/org/apache/commons/jxpath/util/ClassLoaderUtilTest.java @@ -39,8 +39,6 @@ public class ClassLoaderUtilTest extends TestCase { private static final String TEST_CASE_CLASS_NAME = "org.apache.commons.jxpath.util.ClassLoaderUtilTest"; private static final String EXAMPLE_CLASS_NAME = "org.apache.commons.jxpath.util.ClassLoadingExampleClass"; - private static final String DEFAULT_FACTORY_CLASS = "org.apache.commons.jxpath.ri.JXPathContextFactoryReferenceImpl"; - private ClassLoader orginalContextClassLoader; /** @@ -48,7 +46,7 @@ public class ClassLoaderUtilTest extends TestCase { */ @Override public void setUp() { - System.setProperty("jxpath.class.allow", "*"); + System.setProperty("jxpath.class.allow", "java.lang.ObjectXBeanInfo,org.apache.commons.jxpath.*"); this.orginalContextClassLoader = Thread.currentThread().getContextClassLoader(); } @@ -121,7 +119,6 @@ public static void callExampleMessageMethodAndAssertClassNotFoundJXPathException * JXPath and asserts the dynamic class load succeeds. */ public static void callExampleMessageMethodAndAssertSuccess() { - System.setProperty("jxpath.class.allow", EXAMPLE_CLASS_NAME+","+DEFAULT_FACTORY_CLASS); final JXPathContext context = JXPathContext.newContext(new Object()); Object value; try { @@ -129,8 +126,6 @@ public static void callExampleMessageMethodAndAssertSuccess() { assertEquals("an example class", value); } catch( final Exception e ) { fail(e.getMessage()); - } finally { - System.clearProperty("jxpath.class.allow"); } } From 85620c6d2d6a273cc2336edc9d128c2e5949cb98 Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Tue, 25 Oct 2022 16:25:21 +0200 Subject: [PATCH 13/14] Make JXPathFilter accessible/configurable on the context level --- .../commons/jxpath/FunctionLibrary.java | 18 ++++++++++--- .../apache/commons/jxpath/JXPathContext.java | 21 ++++++++++++++++ .../commons/jxpath/PackageFunctions.java | 25 +++++++++++++------ .../jxpath/functions/ConstructorFunction.java | 14 +++++++++++ .../jxpath/functions/MethodFunction.java | 14 +++++++++-- .../jxpath/ri/JXPathContextReferenceImpl.java | 2 +- .../jxpath/ri/SystemPropertyJXPathFilter.java | 3 ++- .../ri/compiler/ExtensionFunctionTest.java | 6 +++-- 8 files changed, 86 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java b/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java index dfd4e75c0..e236fa66d 100644 --- a/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java +++ b/src/main/java/org/apache/commons/jxpath/FunctionLibrary.java @@ -16,6 +16,9 @@ */ package org.apache.commons.jxpath; +import org.apache.commons.jxpath.ri.JXPathFilter; +import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; + import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -74,14 +77,20 @@ public Set getUsedNamespaces() { * @return Function found */ @Override - public Function getFunction(final String namespace, final String name, - final Object[] parameters) { + public Function getFunction(final String namespace, final String name, final Object[] parameters) { + return getFunction(namespace, name, parameters, new SystemPropertyJXPathFilter()); + } + + + @Override + public Function getFunction(final String namespace, final String name, final Object[] parameters, final JXPathFilter filter) { final Object candidates = functionCache().get(namespace); if (candidates instanceof Functions) { return ((Functions) candidates).getFunction( namespace, name, - parameters); + parameters, + filter); } if (candidates instanceof List) { final List list = (List) candidates; @@ -91,7 +100,8 @@ public Function getFunction(final String namespace, final String name, ((Functions) list.get(i)).getFunction( namespace, name, - parameters); + parameters, + filter); if (function != null) { return function; } diff --git a/src/main/java/org/apache/commons/jxpath/JXPathContext.java b/src/main/java/org/apache/commons/jxpath/JXPathContext.java index c4aac2c63..2b2153fad 100644 --- a/src/main/java/org/apache/commons/jxpath/JXPathContext.java +++ b/src/main/java/org/apache/commons/jxpath/JXPathContext.java @@ -23,6 +23,8 @@ import java.util.List; import java.util.Locale; +import org.apache.commons.jxpath.ri.JXPathFilter; +import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; import org.apache.commons.jxpath.util.KeyManagerUtils; /** @@ -422,6 +424,8 @@ public abstract class JXPathContext { /** decimal format map */ protected HashMap decimalFormats; + protected JXPathFilter filter; + private Locale locale; private boolean lenientSet = false; private boolean lenient = false; @@ -469,6 +473,7 @@ private static JXPathContextFactory getContextFactory () { protected JXPathContext(final JXPathContext parentContext, final Object contextBean) { this.parentContext = parentContext; this.contextBean = contextBean; + this.filter = new SystemPropertyJXPathFilter(); } /** @@ -598,6 +603,22 @@ public synchronized Locale getLocale() { return locale; } + /** + * Returns the filter that specifies which classes are allowed. + * @return JXPathFilter + */ + public JXPathFilter getFilter() { + return filter; + } + + /** + * Sets the filter for the context. + * @param filter JXPathFilter + */ + public void setFilter(JXPathFilter filter) { + this.filter = filter; + } + /** * Sets {@link DecimalFormatSymbols} for a given name. The DecimalFormatSymbols * can be referenced as the third, optional argument in the invocation of diff --git a/src/main/java/org/apache/commons/jxpath/PackageFunctions.java b/src/main/java/org/apache/commons/jxpath/PackageFunctions.java index 3a2e4a1ee..93873000f 100644 --- a/src/main/java/org/apache/commons/jxpath/PackageFunctions.java +++ b/src/main/java/org/apache/commons/jxpath/PackageFunctions.java @@ -26,6 +26,8 @@ import org.apache.commons.jxpath.functions.ConstructorFunction; import org.apache.commons.jxpath.functions.MethodFunction; +import org.apache.commons.jxpath.ri.JXPathFilter; +import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; import org.apache.commons.jxpath.util.ClassLoaderUtil; import org.apache.commons.jxpath.util.MethodLookupUtils; import org.apache.commons.jxpath.util.TypeUtils; @@ -116,6 +118,15 @@ public Function getFunction( final String namespace, final String name, Object[] parameters) { + return getFunction(namespace, name, parameters, new SystemPropertyJXPathFilter()); + } + + @Override + public Function getFunction(final String namespace, final String name, Object[] parameters, final JXPathFilter filter) { + if (filter == null) { + throw new JXPathException("No extension function is allowed"); + } + if (!Objects.equals(this.namespace, namespace)) { return null; } @@ -124,6 +135,7 @@ public Function getFunction( parameters = EMPTY_ARRAY; } + String functionName = namespace != null ? namespace + ":" + name : name; if (parameters.length >= 1) { Object target = TypeUtils.convert(parameters[0], Object.class); if (target != null) { @@ -133,7 +145,7 @@ public Function getFunction( name, parameters); if (method != null) { - return new MethodFunction(method); + return new MethodFunction(method, filter); } if (target instanceof NodeSet) { @@ -146,7 +158,7 @@ public Function getFunction( name, parameters); if (method != null) { - return new MethodFunction(method); + return new MethodFunction(method, filter); } if (target instanceof Collection) { @@ -169,7 +181,7 @@ public Function getFunction( name, parameters); if (method != null) { - return new MethodFunction(method); + return new MethodFunction(method, filter); } } } @@ -185,12 +197,11 @@ public Function getFunction( Class functionClass; try { - functionClass = ClassLoaderUtil.getClass(className, true); + functionClass = ClassLoaderUtil.getClass(className, true, filter); } catch (final ClassNotFoundException ex) { throw new JXPathException( - "Cannot invoke extension function " - + (namespace != null ? namespace + ":" + name : name), + "Cannot invoke extension function " + functionName, ex); } @@ -208,7 +219,7 @@ public Function getFunction( methodName, parameters); if (method != null) { - return new MethodFunction(method); + return new MethodFunction(method, filter); } } return null; diff --git a/src/main/java/org/apache/commons/jxpath/functions/ConstructorFunction.java b/src/main/java/org/apache/commons/jxpath/functions/ConstructorFunction.java index 9c07f45fc..05dcc610d 100644 --- a/src/main/java/org/apache/commons/jxpath/functions/ConstructorFunction.java +++ b/src/main/java/org/apache/commons/jxpath/functions/ConstructorFunction.java @@ -22,6 +22,8 @@ import org.apache.commons.jxpath.ExpressionContext; import org.apache.commons.jxpath.Function; import org.apache.commons.jxpath.JXPathInvalidAccessException; +import org.apache.commons.jxpath.ri.JXPathFilter; +import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; import org.apache.commons.jxpath.util.TypeUtils; /** @@ -32,12 +34,19 @@ public class ConstructorFunction implements Function { private final Constructor constructor; + private final JXPathFilter filter; + /** * Create a new ConstructorFunction. * @param constructor the constructor to call. */ public ConstructorFunction(final Constructor constructor) { + this(constructor, new SystemPropertyJXPathFilter()); + } + + public ConstructorFunction(final Constructor constructor, JXPathFilter filter) { this.constructor = constructor; + this.filter = filter; } /** @@ -66,6 +75,11 @@ public Object invoke(final ExpressionContext context, Object[] parameters) { for (int i = 0; i < parameters.length; i++) { args[i + pi] = TypeUtils.convert(parameters[i], types[i + pi]); } + + if (!filter.isClassNameExposed(constructor.getDeclaringClass().getName())) { + throw new Exception("Calling constructors is not allowed for class: " + constructor.getDeclaringClass().getName()); + } + return constructor.newInstance(args); } catch (Throwable ex) { diff --git a/src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java b/src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java index 2b8aaefb0..26579ae95 100644 --- a/src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java +++ b/src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java @@ -36,14 +36,24 @@ public class MethodFunction implements Function { private final Method method; private static final Object[] EMPTY_ARRAY = {}; - private JXPathFilter jxpathFilter = new SystemPropertyJXPathFilter(); + private final JXPathFilter filter; /** * Create a new MethodFunction. * @param method implementing Method */ public MethodFunction(final Method method) { + this(method, new SystemPropertyJXPathFilter()); + } + + /** + * Create a new MethodFunction. + * @param method implementing Method + * @param filter JXPathFilter + */ + public MethodFunction(final Method method, final JXPathFilter filter) { this.method = ValueUtils.getAccessibleMethod(method); + this.filter = filter; } @Override @@ -92,7 +102,7 @@ public Object invoke(final ExpressionContext context, Object[] parameters) { } } - if (!jxpathFilter.isClassNameExposed(method.getDeclaringClass().getName())) { + if (!filter.isClassNameExposed(method.getDeclaringClass().getName())) { throw new Exception("Calling method is not allowed: " + method.getDeclaringClass() + "." + method.getName() + "()"); } return method.invoke(target, args); diff --git a/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java b/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java index 2d7319944..08444f8b9 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java +++ b/src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java @@ -765,7 +765,7 @@ public Function getFunction(final QName functionName, final Object[] parameters) while (funcCtx != null) { funcs = funcCtx.getFunctions(); if (funcs != null) { - func = funcs.getFunction(namespace, name, parameters); + func = funcs.getFunction(namespace, name, parameters, filter); if (func != null) { return func; } diff --git a/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java index 461931e55..5fa83aca1 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java @@ -29,7 +29,8 @@ public class SystemPropertyJXPathFilter implements JXPathFilter { public SystemPropertyJXPathFilter() { final String allowedClasses = System.getProperty("jxpath.class.allow"); - List allowedClassesList = (allowedClasses != null && !allowedClasses.isEmpty()) + List allowedClassesList = + allowedClasses != null && !allowedClasses.isEmpty() ? Arrays.asList(allowedClasses.split(",")) : Collections.emptyList(); this.allowedClasses = Collections.unmodifiableSet(new HashSet<>(allowedClassesList)); diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java index fb4d13851..5e20137d3 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java @@ -33,6 +33,7 @@ import org.apache.commons.jxpath.Function; import org.apache.commons.jxpath.ExpressionContext; import org.apache.commons.jxpath.Pointer; +import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; import org.apache.commons.jxpath.ri.model.NodePointer; import org.apache.commons.jxpath.util.JXPath11CompatibleTypeConverter; import org.apache.commons.jxpath.util.TypeConverter; @@ -472,17 +473,18 @@ public void testJXPathContextFunctionsWithoutClassFilter() { public void testJXPathContextFunctionsWithClassFilter() { try { System.setProperty("jxpath.class.allow", "java.lang.Thread"); + context.setFilter(new SystemPropertyJXPathFilter()); long startTime = System.currentTimeMillis(); context.iterate("java.lang.Thread.sleep(5)"); assertTrue(System.currentTimeMillis() >= startTime + 5); startTime = System.currentTimeMillis(); - context.selectSingleNode("java.lang.Thread.sleep(50)"); + context.selectSingleNode("java.lang.Thread.sleep(5)"); assertTrue(System.currentTimeMillis() >= startTime + 5); } catch (Throwable t) { fail(t.getMessage()); } finally { - System.clearProperty("jxpath.class.allow"); + System.setProperty("jxpath.class.allow", DEFAULT_ALLOW_LIST); } } From c9e1b305cbba23645a4318789c2920fabdaa89e8 Mon Sep 17 00:00:00 2001 From: Khaled Yakdan Date: Tue, 25 Oct 2022 17:25:17 +0200 Subject: [PATCH 14/14] Add a CustomJXPathFilter class for programmatically configurable filter --- .../jxpath/ri/AbstractJXPathFilter.java | 56 +++++++++++++++++++ .../commons/jxpath/ri/CustomJXPathFilter.java | 31 ++++++++++ .../jxpath/ri/SystemPropertyJXPathFilter.java | 53 ++++++------------ .../ri/compiler/ExtensionFunctionTest.java | 53 ++++++++++++++++-- 4 files changed, 151 insertions(+), 42 deletions(-) create mode 100644 src/main/java/org/apache/commons/jxpath/ri/AbstractJXPathFilter.java create mode 100644 src/main/java/org/apache/commons/jxpath/ri/CustomJXPathFilter.java diff --git a/src/main/java/org/apache/commons/jxpath/ri/AbstractJXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/AbstractJXPathFilter.java new file mode 100644 index 000000000..b4ec9affc --- /dev/null +++ b/src/main/java/org/apache/commons/jxpath/ri/AbstractJXPathFilter.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.jxpath.ri; + +import java.util.Objects; +import java.util.Set; + +public class AbstractJXPathFilter implements JXPathFilter { + protected final Set allowedClasses; + + public AbstractJXPathFilter(final Set allowedClasses) { + this.allowedClasses = allowedClasses; + } + + /** + * Specifies whether the Java class of the specified name be exposed via xpath + * + * @param className is the fully qualified name of the java class being checked. + * This will not be null. Only non-array class names will be passed. + * @return true if the java class can be exposed via xpath, false otherwise + */ + @Override + public boolean isClassNameExposed(String className) { + if (allowedClasses.isEmpty()) { + return false; + } + + if (allowedClasses.contains("*")) { + return true; + } + + return allowedClasses.stream().anyMatch(pattern -> { + if (Objects.equals(className, pattern)) { + return true; + } + else if (pattern.endsWith("*")) { + return className.startsWith(pattern.substring(0, pattern.length() - 1)); + } + return false; + }); + } +} diff --git a/src/main/java/org/apache/commons/jxpath/ri/CustomJXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/CustomJXPathFilter.java new file mode 100644 index 000000000..d28e56c8c --- /dev/null +++ b/src/main/java/org/apache/commons/jxpath/ri/CustomJXPathFilter.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.jxpath.ri; + +import java.util.Collections; +import java.util.Set; + +/** + * A filter to be used by JXPath based on a configurable custom list of pattern + * provided to the constructor. + */ +public class CustomJXPathFilter extends AbstractJXPathFilter { + + public CustomJXPathFilter(Set allowedClasses) { + super(Collections.unmodifiableSet(allowedClasses)); + } +} diff --git a/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java b/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java index 5fa83aca1..5e34a1b1c 100644 --- a/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java +++ b/src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java @@ -16,50 +16,29 @@ */ package org.apache.commons.jxpath.ri; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; /** - * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions. - * This class implements specific filter interfaces, and implements methods in those. - * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath - * JXPath uses this filter instance when an extension function instance is created. + * A filter to be used by JXPath based on the system property jxpath.class.allow. + * This property is a comma separated list of patterns defining which classes + * are allowed to be loaded by JXPath. */ -public class SystemPropertyJXPathFilter implements JXPathFilter { - private final Set allowedClasses; +public class SystemPropertyJXPathFilter extends AbstractJXPathFilter { public SystemPropertyJXPathFilter() { + super(loadAllowedClassesFromSystemProperty()); + } + + private static Set loadAllowedClassesFromSystemProperty() { final String allowedClasses = System.getProperty("jxpath.class.allow"); List allowedClassesList = allowedClasses != null && !allowedClasses.isEmpty() - ? Arrays.asList(allowedClasses.split(",")) - : Collections.emptyList(); - this.allowedClasses = Collections.unmodifiableSet(new HashSet<>(allowedClassesList)); - } - - /** - * Specifies whether the Java class of the specified name be exposed via xpath - * - * @param className is the fully qualified name of the java class being checked. - * This will not be null. Only non-array class names will be passed. - * @return true if the java class can be exposed via xpath, false otherwise - */ - @Override - public boolean isClassNameExposed(String className) { - if (allowedClasses.isEmpty()) { - return false; - } - - if (allowedClasses.contains("*")) { - return true; - } - - return allowedClasses.stream().anyMatch(pattern -> { - if (Objects.equals(className, pattern)) { - return true; - } else if (pattern.endsWith("*")) { - return className.startsWith(pattern.substring(0, pattern.length() - 1)); - } - return false; - }); + ? Arrays.asList(allowedClasses.split(",")) + : Collections.emptyList(); + return Collections.unmodifiableSet(new HashSet<>(allowedClassesList)); } } diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java index 5e20137d3..fa04c6ef5 100644 --- a/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java +++ b/src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java @@ -16,10 +16,7 @@ */ package org.apache.commons.jxpath.ri.compiler; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Locale; +import java.util.*; import org.apache.commons.jxpath.JXPathTestCase; import org.apache.commons.jxpath.Functions; @@ -33,6 +30,7 @@ import org.apache.commons.jxpath.Function; import org.apache.commons.jxpath.ExpressionContext; import org.apache.commons.jxpath.Pointer; +import org.apache.commons.jxpath.ri.CustomJXPathFilter; import org.apache.commons.jxpath.ri.SystemPropertyJXPathFilter; import org.apache.commons.jxpath.ri.model.NodePointer; import org.apache.commons.jxpath.util.JXPath11CompatibleTypeConverter; @@ -470,7 +468,7 @@ public void testJXPathContextFunctionsWithoutClassFilter() { } } - public void testJXPathContextFunctionsWithClassFilter() { + public void testJXPathContextFunctionsWithSystemPropertyFilter() { try { System.setProperty("jxpath.class.allow", "java.lang.Thread"); context.setFilter(new SystemPropertyJXPathFilter()); @@ -488,6 +486,51 @@ public void testJXPathContextFunctionsWithClassFilter() { } } + public void testJXPathAllowContextFunctionsWithCustomFilter() { + try { + System.clearProperty("jxpath.class.allow"); + context.setFilter(new CustomJXPathFilter(new HashSet<>(Collections.singletonList("java.lang.Thread")))); + long startTime = System.currentTimeMillis(); + context.iterate("java.lang.Thread.sleep(5)"); + assertTrue(System.currentTimeMillis() >= startTime + 5); + + startTime = System.currentTimeMillis(); + context.selectSingleNode("java.lang.Thread.sleep(5)"); + assertTrue(System.currentTimeMillis() >= startTime + 5); + } catch (Throwable t) { + fail(t.getMessage()); + } finally { + System.setProperty("jxpath.class.allow", DEFAULT_ALLOW_LIST); + } + } + + public void testJXPathDenyContextFunctionsWithCustomFilter() { + System.setProperty("jxpath.class.allow", "*"); + context.setFilter(new CustomJXPathFilter(Collections.emptySet())); + try { + context.iterate("java.lang.Thread.sleep(5)"); + throw new Exception("testJXPathDenyContextFunctionsWithCustomFilter() failed for iterate()"); + } catch (Throwable t) { + if (!(t.getMessage().contains("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread")) + && !(t.getMessage().contains("java.lang.ClassNotFoundException: java.lang.Thread"))) { + fail("failed to deny calling java.lang.Thread.sleep(5)"); + } + } + + context.setFilter(new CustomJXPathFilter(Collections.emptySet())); + try { + context.selectSingleNode("java.lang.Thread.sleep(5)"); + throw new Exception("testJXPathDenyContextFunctionsWithCustomFilter() failed for iterate()"); + } catch (Throwable t) { + if (!(t.getMessage().contains("Cannot invoke extension function java.lang.Thread.sleep; java.lang.Thread")) + && !(t.getMessage().contains("java.lang.ClassNotFoundException: java.lang.Thread"))) { + fail("failed to deny calling java.lang.Thread.sleep(5)"); + } + } + + System.setProperty("jxpath.class.allow", DEFAULT_ALLOW_LIST); + } + private static class Context implements ExpressionContext { private final Object object;