Merge pull request #26 from arungitan/1.7.x-patch
[Backport] Velocity 931 update secure classlist (updated)
diff --git a/src/java/org/apache/velocity/runtime/defaults/velocity.properties b/src/java/org/apache/velocity/runtime/defaults/velocity.properties
index 750a59a..2eb8b72 100644
--- a/src/java/org/apache/velocity/runtime/defaults/velocity.properties
+++ b/src/java/org/apache/velocity/runtime/defaults/velocity.properties
@@ -245,15 +245,15 @@
# accessed.
# ----------------------------------------------------------------------------
+# Prohibit reflection
introspector.restrict.packages = java.lang.reflect
-# The two most dangerous classes
+# ClassLoader, Thread, and subclasses disabled by default in SecureIntrospectorImpl
+
+# 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.ClassLoader
-
-# Restrict these for extra safety
-
introspector.restrict.classes = java.lang.Compiler
introspector.restrict.classes = java.lang.InheritableThreadLocal
introspector.restrict.classes = java.lang.Package
@@ -262,8 +262,14 @@
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
+
diff --git a/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java b/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
index 398efe0..33098d2 100644
--- a/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
+++ b/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
@@ -87,30 +87,30 @@
*/
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 @@
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/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java b/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java
index f227089..3e3af33 100644
--- a/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java
+++ b/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java
@@ -37,6 +37,9 @@
import org.apache.velocity.runtime.RuntimeConstants;
import org.apache.velocity.util.introspection.SecureUberspector;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
/**
* Checks that the secure introspector is working properly.
*
@@ -117,16 +120,16 @@
try
{
- for (int i=0; i < templateStrings.length; i++)
+ for (String templateString : templateStrings)
{
- if (shouldeval && !doesStringEvaluate(ve,c,templateStrings[i]))
+ if (shouldeval && !doesStringEvaluate(ve, c, templateString))
{
- fail ("Should have evaluated: " + templateStrings[i]);
+ fail("Should have evaluated: " + templateString);
}
- if (!shouldeval && doesStringEvaluate(ve,c,templateStrings[i]))
+ if (!shouldeval && doesStringEvaluate(ve, c, templateString))
{
- fail ("Should not have evaluated: " + templateStrings[i]);
+ fail("Should not have evaluated: " + templateString);
}
}
@@ -139,9 +142,9 @@
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);
@@ -164,14 +167,35 @@
}
- 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);
+ }
+ }
}