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);
+        }
+    }
 
 }