diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java b/velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java index 38ac7ce85e..65aa04cfe8 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java @@ -87,30 +87,30 @@ public Method getMethod(Class clazz, String methodName, Object[] params) */ public boolean checkObjectExecutePermission(Class clazz, String methodName) { - /** - * check for wait and notify - */ + /** + * check for wait and notify + */ if (methodName != null && (methodName.equals("wait") || methodName.equals("notify")) ) - { - return false; - } + { + return false; + } - /** - * Always allow the most common classes - Number, Boolean and String - */ - else if (Number.class.isAssignableFrom(clazz)) - { - return true; - } - else if (Boolean.class.isAssignableFrom(clazz)) - { - return true; - } - else if (String.class.isAssignableFrom(clazz)) - { - return true; - } + /** + * Always allow the most common classes - Number, Boolean and String + */ + else if (Number.class.isAssignableFrom(clazz)) + { + return true; + } + else if (Boolean.class.isAssignableFrom(clazz)) + { + return true; + } + else if (String.class.isAssignableFrom(clazz)) + { + return true; + } /** * Always allow Class.getName() @@ -121,6 +121,15 @@ else if (Class.class.isAssignableFrom(clazz) && return true; } + /** + * Always disallow ClassLoader, Thread and subclasses + */ + if (ClassLoader.class.isAssignableFrom(clazz) || + Thread.class.isAssignableFrom(clazz)) + { + return false; + } + /** * check the classname (minus any array info) * whether it matches disallowed classes or packages diff --git a/velocity-engine-core/src/test/java/org/apache/velocity/test/SecureIntrospectionTestCase.java b/velocity-engine-core/src/test/java/org/apache/velocity/test/SecureIntrospectionTestCase.java index 8653f40aa2..081e48b248 100644 --- a/velocity-engine-core/src/test/java/org/apache/velocity/test/SecureIntrospectionTestCase.java +++ b/velocity-engine-core/src/test/java/org/apache/velocity/test/SecureIntrospectionTestCase.java @@ -33,6 +33,9 @@ import java.io.IOException; import java.io.StringWriter; import java.io.Writer; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; import java.util.Collection; import java.util.HashSet; @@ -138,9 +141,9 @@ private void doTestMethods(VelocityEngine ve, String[] templateStrings, boolean private boolean doesStringEvaluate(VelocityEngine ve, Context c, String inputString) throws ParseErrorException, MethodInvocationException, ResourceNotFoundException, IOException { - // assume that an evaluation is bad if the input and result are the same (e.g. a bad reference) - // or the result is an empty string (e.g. bad #foreach) - Writer w = new StringWriter(); + // assume that an evaluation is bad if the input and result are the same (e.g. a bad reference) + // or the result is an empty string (e.g. bad #foreach) + Writer w = new StringWriter(); ve.evaluate(c, w, "foo", inputString); String result = w.toString(); return (result.length() > 0 ) && !result.equals(inputString); @@ -163,14 +166,35 @@ public void setProperty(String val) } - public Collection getCollection() - { - Collection c = new HashSet(); - c.add("aaa"); - c.add("bbb"); - c.add("ccc"); - return c; - } + public Collection getCollection() + { + Collection c = new HashSet(); + c.add("aaa"); + c.add("bbb"); + c.add("ccc"); + return c; + } + + public ClassLoader getSampleClassLoader1() + { + return this.getClass().getClassLoader(); + } + + /** + * sample property which is a subclass of ClassLoader + * @return + */ + public ClassLoader getSampleClassLoader2() + { + try + { + return new URLClassLoader(new URL[]{new URL("file://.")}, this.getClass().getClassLoader()); + } + catch (MalformedURLException e) + { + throw new RuntimeException(e); + } + } } diff --git a/velocity-engine-core/src/test/resources/oldproperties/velocity.properties b/velocity-engine-core/src/test/resources/oldproperties/velocity.properties index b3b61a6ed9..ce487db091 100644 --- a/velocity-engine-core/src/test/resources/oldproperties/velocity.properties +++ b/velocity-engine-core/src/test/resources/oldproperties/velocity.properties @@ -220,15 +220,15 @@ runtime.conversion.handler.class = org.apache.velocity.util.introspection.TypeCo # accessed. # ---------------------------------------------------------------------------- +# Prohibit reflection introspector.restrict.packages = java.lang.reflect -# The two most dangerous classes +# ClassLoader, Thread, and subclasses disabled by default in SecureIntrospectorImpl -introspector.restrict.classes = java.lang.Class -introspector.restrict.classes = java.lang.ClassLoader - -# Restrict these for extra safety +# Restrict these system classes. Note that anything in this list is matched exactly. +# (Subclasses must be explicitly named to be included). +introspector.restrict.classes = java.lang.Class introspector.restrict.classes = java.lang.Compiler introspector.restrict.classes = java.lang.InheritableThreadLocal introspector.restrict.classes = java.lang.Package @@ -237,10 +237,16 @@ introspector.restrict.classes = java.lang.Runtime introspector.restrict.classes = java.lang.RuntimePermission introspector.restrict.classes = java.lang.SecurityManager introspector.restrict.classes = java.lang.System -introspector.restrict.classes = java.lang.Thread introspector.restrict.classes = java.lang.ThreadGroup introspector.restrict.classes = java.lang.ThreadLocal +# Restrict instance managers for common servlet containers (Tomcat, JBoss, Jetty) + +introspector.restrict.classes = org.apache.catalina.core.DefaultInstanceManager +introspector.restrict.classes = org.apache.tomcat.SimpleInstanceManager +introspector.restrict.classes = org.wildfly.extension.undertow.deployment.UndertowJSPInstanceManager +introspector.restrict.classes = org.eclipse.jetty.util.DecoratedObjectFactory + # ---------------------------------------------------------------------------- # SPACE GOBBLING # ----------------------------------------------------------------------------