This is an automated email from the ASF dual-hosted git repository.
cbrisson pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/velocity-engine.git
The following commit(s) were added to refs/heads/1.7.x by this push:
new bc9220d2 Backport for Velocity 931 (update secure classlist)
new 267fc84f Merge pull request #26 from arungitan/1.7.x-patch
bc9220d2 is described below
commit bc9220d26aa5ad4b9b364c228b579ca5d5ed00fb
Author: Will Glass-Husain <[email protected]>
AuthorDate: Thu Jul 16 21:59:43 2020 -0700
Backport for Velocity 931 (update secure classlist)
Contains:
CR: remove edits unrelated to patch
add further tomcat class to restricted list
(cherry picked from commit 3f5d477bb4f4397bed2d2926c35dcef7de3aae3e)
update list of restricted classes
(cherry picked from commit 15909056fe51f5d39d49e101d706d3075876dde4)
tests for SecureIntrospection
(cherry picked from commit 3e28c5fb7e618dc002cdbc6e4d0df9e2fd4bc155)
whitespace
(cherry picked from commit 2ce3d7cd829a9f834b2a01809d0c4734054ac2c8)
disallow ClassLoader, Thread, and subclasses.
(cherry picked from commit 1ba60771d23dae7e6b3138ae6bee09cf6f9d2485)
whitespace fix
(cherry picked from commit ad9109b40ca20d48955003dc8b0e5b669901ba7b)
---
.../velocity/runtime/defaults/velocity.properties | 18 ++++---
.../util/introspection/SecureIntrospectorImpl.java | 51 ++++++++++++--------
.../velocity/test/SecureIntrospectionTestCase.java | 56 +++++++++++++++-------
3 files changed, 82 insertions(+), 43 deletions(-)
diff --git a/src/java/org/apache/velocity/runtime/defaults/velocity.properties
b/src/java/org/apache/velocity/runtime/defaults/velocity.properties
index 750a59af..2eb8b722 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 @@ runtime.introspector.uberspect =
org.apache.velocity.util.introspection.Uberspec
# 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
@@ -262,8 +262,14 @@ 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
+
diff --git
a/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
b/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
index 398efe0e..33098d24 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 class SecureIntrospectorImpl extends Introspector
implements SecureIntros
*/
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 @@ public class SecureIntrospectorImpl extends Introspector
implements SecureIntros
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 f227089f..3e3af339 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.exception.ResourceNotFoundException;
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 @@ public class SecureIntrospectionTestCase extends
BaseTestCase
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 @@ public class SecureIntrospectionTestCase extends
BaseTestCase
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 class SecureIntrospectionTestCase extends
BaseTestCase
}
- 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);
+ }
+ }
}
