You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Nathan Bubna <nb...@gmail.com> on 2007/02/19 16:36:08 UTC
Re: svn commit: r509094 - in /velocity/engine/trunk/src: java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java test/org/apache/velocity/test/SecureIntrospectionTestCase.java
Awful lot of formatting changes mixed in here, makes it hard to tell
what changed. :(
On 2/18/07, wglass@apache.org <wg...@apache.org> wrote:
> Author: wglass
> Date: Sun Feb 18 21:17:09 2007
> New Revision: 509094
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=509094
> Log:
> Allow SecureUberspector to work with iterators in #foreach. Fixes VELOCITY-516.
>
> Modified:
> velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
> velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java
>
> Modified: velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java?view=diff&rev=509094&r1=509093&r2=509094
> ==============================================================================
> --- velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java (original)
> +++ velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java Sun Feb 18 21:17:09 2007
> @@ -16,7 +16,7 @@
> * "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.
> + * under the License.
> */
>
> import java.lang.reflect.Method;
> @@ -24,14 +24,14 @@
> import org.apache.velocity.runtime.log.Log;
>
> /**
> - * <p>Prevent "dangerous" classloader/reflection related calls. Use this
> + * <p>Prevent "dangerous" classloader/reflection related calls. Use this
> * introspector for situations in which template writers are numerous
> * or untrusted. Specifically, this introspector prevents creation of
> * arbitrary objects and prevents reflection on objects.
> - *
> - * <p>See documentation of checkObjectExecutePermission() for
> + *
> + * <p>See documentation of checkObjectExecutePermission() for
> * more information on specific classes and methods blocked.
> - *
> + *
> * @author <a href="mailto:wglass@forio.com">Will Glass-Husain</a>
> * @version $Id$
> */
> @@ -39,19 +39,19 @@
> {
> private String[] badClasses;
> private String[] badPackages;
> -
> +
> public SecureIntrospectorImpl(String[] badClasses, String[] badPackages, Log log)
> {
> super(log);
> this.badClasses = badClasses;
> this.badPackages = badPackages;
> }
> -
> +
> /**
> - * Get the Method object corresponding to the given class, name and parameters.
> + * Get the Method object corresponding to the given class, name and parameters.
> * Will check for appropriate execute permissions and return null if the method
> * is not allowed to be executed.
> - *
> + *
> * @param clazz Class on which method will be called
> * @param methodName Name of method to be called
> * @param params array of parameters to method
> @@ -62,84 +62,81 @@
> {
> if (!checkObjectExecutePermission(clazz,methodName))
> {
> - log.warn ("Cannot retrieve method " + methodName +
> + log.warn ("Cannot retrieve method " + methodName +
> " from object of class " + clazz.getName() +
> " due to security restrictions.");
> return null;
> -
> +
> }
> else
> {
> return super.getMethod(clazz, methodName, params);
> }
> }
> -
> +
> /**
> * Determine which methods and classes to prevent from executing. Always blocks
> * methods wait() and notify(). Always allows methods on Number, Boolean, and String.
> * Prohibits method calls on classes related to reflection and system operations.
> * For the complete list, see the properties <code>introspector.restrict.classes</code>
> * and <code>introspector.restrict.packages</code>.
> - *
> + *
> * @param clazz Class on which method will be called
> * @param methodName Name of method to be called
> * @see org.apache.velocity.util.introspection.SecureIntrospectorControl#checkObjectExecutePermission(java.lang.Class, java.lang.String)
> */
> public boolean checkObjectExecutePermission(Class clazz, String methodName)
> {
> - if (methodName == null)
> - {
> - return false;
> - }
> -
> - /**
> - * check for wait and notify
> - */
> - if ( methodName.equals("wait") || methodName.equals("notify") )
> - {
> - return false;
> - }
> -
> - /**
> - * Always allow the most common classes - Number, Boolean and String
> - */
> - else if (java.lang.Number.class.isAssignableFrom(clazz))
> - {
> - return true;
> - }
> -
> - else if (java.lang.Boolean.class.isAssignableFrom(clazz))
> - {
> - return true;
> - }
> -
> - else if (java.lang.String.class.isAssignableFrom(clazz))
> - {
> - return true;
> - }
> -
> +
> + /**
> + * check for wait and notify
> + */
> + if ( (methodName != null) && (methodName.equals("wait") || methodName.equals("notify")) )
> + {
> + return false;
> + }
> +
> + /**
> + * Always allow the most common classes - Number, Boolean and String
> + */
> + else if (java.lang.Number.class.isAssignableFrom(clazz))
> + {
> + return true;
> + }
> +
> + else if (java.lang.Boolean.class.isAssignableFrom(clazz))
> + {
> + return true;
> + }
> +
> + else if (java.lang.String.class.isAssignableFrom(clazz))
> + {
> + return true;
> + }
> +
> +
> /**
> * Always allow Class.getName()
> */
> - else if (java.lang.Class.class.isAssignableFrom(clazz) && methodName.equals("getName"))
> + else if (java.lang.Class.class.isAssignableFrom(clazz) && (methodName != null) && methodName.equals("getName"))
> {
> return true;
> }
> -
> +
> /**
> * check the classname (minus any array info)
> * whether it matches disallowed classes or packages
> - */
> + */
> String className = clazz.getName();
> if (className.startsWith("[L") && className.endsWith(";"))
> {
> className = className.substring(2,className.length() - 1);
> }
> -
> +
> String packageName;
> int dotPos = className.lastIndexOf('.');
> packageName = (dotPos == -1) ? "" : className.substring(0,dotPos);
> -
> +
> int sz = badPackages.length;
> for (int i = 0; i < sz; i++)
> {
> @@ -148,7 +145,7 @@
> return false;
> }
> }
> -
> +
> sz = badClasses.length;
> for (int i = 0; i < sz; i++)
> {
> @@ -157,7 +154,7 @@
> return false;
> }
> }
> -
> +
> return true;
> }
> }
>
> Modified: velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java?view=diff&rev=509094&r1=509093&r2=509094
> ==============================================================================
> --- velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java (original)
> +++ velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java Sun Feb 18 21:17:09 2007
> @@ -16,12 +16,14 @@
> * "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.
> + * under the License.
> */
>
> import java.io.IOException;
> import java.io.StringWriter;
> import java.io.Writer;
> +import java.util.Collection;
> +import java.util.HashSet;
>
> import junit.framework.Test;
> import junit.framework.TestSuite;
> @@ -46,12 +48,13 @@
>
> /**
> * Default constructor.
> + * @param name
> */
> public SecureIntrospectionTestCase(String name)
> {
> super(name);
> }
> -
> +
> public static Test suite()
> {
> return new TestSuite(SecureIntrospectionTestCase.class);
> @@ -67,13 +70,15 @@
>
> private String [] goodTemplateStrings =
> {
> + "#foreach($item in $test.collection)$item#end",
> "$test.Class.Name",
> "#set($test.Property = 'abc')$test.Property",
> "$test.aTestMethod()"
> };
>
> /**
> - * Test to see that "dangerous" methods are forbidden
> + * Test to see that "dangerous" methods are forbidden
> + * @exception Exception
> */
> public void testBadMethodCalls()
> throws Exception
> @@ -89,7 +94,8 @@
> }
>
> /**
> - * Test to see that "dangerous" methods are forbidden
> + * Test to see that "dangerous" methods are forbidden
> + * @exception Exception
> */
> public void testGoodMethodCalls()
> throws Exception
> @@ -109,7 +115,7 @@
> Context c = new VelocityContext();
> c.put("test", this);
>
> - try
> + try
> {
> for (int i=0; i < templateStrings.length; i++)
> {
> @@ -117,15 +123,15 @@
> {
> fail ("Should have evaluated: " + templateStrings[i]);
> }
> -
> +
> if (!shouldeval && doesStringEvaluate(ve,c,templateStrings[i]))
> {
> fail ("Should not have evaluated: " + templateStrings[i]);
> }
> }
>
> - }
> - catch (Exception e)
> + }
> + catch (Exception e)
> {
> fail(e.toString());
> }
> @@ -133,10 +139,12 @@
>
> private boolean doesStringEvaluate(VelocityEngine ve, Context c, String inputString) throws ParseErrorException, MethodInvocationException, ResourceNotFoundException, IOException
> {
> - 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.equals(inputString);
> + return (result.length() > 0 ) && !result.equals(inputString);
> }
>
> private String testProperty;
> @@ -144,14 +152,27 @@
> {
> return testProperty;
> }
> -
> +
> public int aTestMethod()
> {
> return 1;
> }
> -
> +
> public void setProperty(String val)
> {
> testProperty = val;
> }
> +
> +
> + public Collection getCollection()
> + {
> + Collection c = new HashSet();
> + c.add("aaa");
> + c.add("bbb");
> + c.add("ccc");
> + return c;
> + }
> +
> }
> +
> +
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r509094 - in /velocity/engine/trunk/src: java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java test/org/apache/velocity/test/SecureIntrospectionTestCase.java
Posted by Will Glass-Husain <wg...@gmail.com>.
noticed that too late. will do an "svn status" next time. Might be
changing tabs to spaces (my editor does that automatically).
WILL
On 2/19/07, Nathan Bubna <nb...@gmail.com> wrote:
> Awful lot of formatting changes mixed in here, makes it hard to tell
> what changed. :(
>
> On 2/18/07, wglass@apache.org <wg...@apache.org> wrote:
> > Author: wglass
> > Date: Sun Feb 18 21:17:09 2007
> > New Revision: 509094
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=509094
> > Log:
> > Allow SecureUberspector to work with iterators in #foreach. Fixes VELOCITY-516.
> >
> > Modified:
> > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
> > velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java
> >
> > Modified: velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java
> > URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java?view=diff&rev=509094&r1=509093&r2=509094
> > ==============================================================================
> > --- velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java (original)
> > +++ velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/SecureIntrospectorImpl.java Sun Feb 18 21:17:09 2007
> > @@ -16,7 +16,7 @@
> > * "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.
> > + * under the License.
> > */
> >
> > import java.lang.reflect.Method;
> > @@ -24,14 +24,14 @@
> > import org.apache.velocity.runtime.log.Log;
> >
> > /**
> > - * <p>Prevent "dangerous" classloader/reflection related calls. Use this
> > + * <p>Prevent "dangerous" classloader/reflection related calls. Use this
> > * introspector for situations in which template writers are numerous
> > * or untrusted. Specifically, this introspector prevents creation of
> > * arbitrary objects and prevents reflection on objects.
> > - *
> > - * <p>See documentation of checkObjectExecutePermission() for
> > + *
> > + * <p>See documentation of checkObjectExecutePermission() for
> > * more information on specific classes and methods blocked.
> > - *
> > + *
> > * @author <a href="mailto:wglass@forio.com">Will Glass-Husain</a>
> > * @version $Id$
> > */
> > @@ -39,19 +39,19 @@
> > {
> > private String[] badClasses;
> > private String[] badPackages;
> > -
> > +
> > public SecureIntrospectorImpl(String[] badClasses, String[] badPackages, Log log)
> > {
> > super(log);
> > this.badClasses = badClasses;
> > this.badPackages = badPackages;
> > }
> > -
> > +
> > /**
> > - * Get the Method object corresponding to the given class, name and parameters.
> > + * Get the Method object corresponding to the given class, name and parameters.
> > * Will check for appropriate execute permissions and return null if the method
> > * is not allowed to be executed.
> > - *
> > + *
> > * @param clazz Class on which method will be called
> > * @param methodName Name of method to be called
> > * @param params array of parameters to method
> > @@ -62,84 +62,81 @@
> > {
> > if (!checkObjectExecutePermission(clazz,methodName))
> > {
> > - log.warn ("Cannot retrieve method " + methodName +
> > + log.warn ("Cannot retrieve method " + methodName +
> > " from object of class " + clazz.getName() +
> > " due to security restrictions.");
> > return null;
> > -
> > +
> > }
> > else
> > {
> > return super.getMethod(clazz, methodName, params);
> > }
> > }
> > -
> > +
> > /**
> > * Determine which methods and classes to prevent from executing. Always blocks
> > * methods wait() and notify(). Always allows methods on Number, Boolean, and String.
> > * Prohibits method calls on classes related to reflection and system operations.
> > * For the complete list, see the properties <code>introspector.restrict.classes</code>
> > * and <code>introspector.restrict.packages</code>.
> > - *
> > + *
> > * @param clazz Class on which method will be called
> > * @param methodName Name of method to be called
> > * @see org.apache.velocity.util.introspection.SecureIntrospectorControl#checkObjectExecutePermission(java.lang.Class, java.lang.String)
> > */
> > public boolean checkObjectExecutePermission(Class clazz, String methodName)
> > {
> > - if (methodName == null)
> > - {
> > - return false;
> > - }
> > -
> > - /**
> > - * check for wait and notify
> > - */
> > - if ( methodName.equals("wait") || methodName.equals("notify") )
> > - {
> > - return false;
> > - }
> > -
> > - /**
> > - * Always allow the most common classes - Number, Boolean and String
> > - */
> > - else if (java.lang.Number.class.isAssignableFrom(clazz))
> > - {
> > - return true;
> > - }
> > -
> > - else if (java.lang.Boolean.class.isAssignableFrom(clazz))
> > - {
> > - return true;
> > - }
> > -
> > - else if (java.lang.String.class.isAssignableFrom(clazz))
> > - {
> > - return true;
> > - }
> > -
> > +
> > + /**
> > + * check for wait and notify
> > + */
> > + if ( (methodName != null) && (methodName.equals("wait") || methodName.equals("notify")) )
> > + {
> > + return false;
> > + }
> > +
> > + /**
> > + * Always allow the most common classes - Number, Boolean and String
> > + */
> > + else if (java.lang.Number.class.isAssignableFrom(clazz))
> > + {
> > + return true;
> > + }
> > +
> > + else if (java.lang.Boolean.class.isAssignableFrom(clazz))
> > + {
> > + return true;
> > + }
> > +
> > + else if (java.lang.String.class.isAssignableFrom(clazz))
> > + {
> > + return true;
> > + }
> > +
> > +
> > /**
> > * Always allow Class.getName()
> > */
> > - else if (java.lang.Class.class.isAssignableFrom(clazz) && methodName.equals("getName"))
> > + else if (java.lang.Class.class.isAssignableFrom(clazz) && (methodName != null) && methodName.equals("getName"))
> > {
> > return true;
> > }
> > -
> > +
> > /**
> > * check the classname (minus any array info)
> > * whether it matches disallowed classes or packages
> > - */
> > + */
> > String className = clazz.getName();
> > if (className.startsWith("[L") && className.endsWith(";"))
> > {
> > className = className.substring(2,className.length() - 1);
> > }
> > -
> > +
> > String packageName;
> > int dotPos = className.lastIndexOf('.');
> > packageName = (dotPos == -1) ? "" : className.substring(0,dotPos);
> > -
> > +
> > int sz = badPackages.length;
> > for (int i = 0; i < sz; i++)
> > {
> > @@ -148,7 +145,7 @@
> > return false;
> > }
> > }
> > -
> > +
> > sz = badClasses.length;
> > for (int i = 0; i < sz; i++)
> > {
> > @@ -157,7 +154,7 @@
> > return false;
> > }
> > }
> > -
> > +
> > return true;
> > }
> > }
> >
> > Modified: velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java
> > URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java?view=diff&rev=509094&r1=509093&r2=509094
> > ==============================================================================
> > --- velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java (original)
> > +++ velocity/engine/trunk/src/test/org/apache/velocity/test/SecureIntrospectionTestCase.java Sun Feb 18 21:17:09 2007
> > @@ -16,12 +16,14 @@
> > * "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.
> > + * under the License.
> > */
> >
> > import java.io.IOException;
> > import java.io.StringWriter;
> > import java.io.Writer;
> > +import java.util.Collection;
> > +import java.util.HashSet;
> >
> > import junit.framework.Test;
> > import junit.framework.TestSuite;
> > @@ -46,12 +48,13 @@
> >
> > /**
> > * Default constructor.
> > + * @param name
> > */
> > public SecureIntrospectionTestCase(String name)
> > {
> > super(name);
> > }
> > -
> > +
> > public static Test suite()
> > {
> > return new TestSuite(SecureIntrospectionTestCase.class);
> > @@ -67,13 +70,15 @@
> >
> > private String [] goodTemplateStrings =
> > {
> > + "#foreach($item in $test.collection)$item#end",
> > "$test.Class.Name",
> > "#set($test.Property = 'abc')$test.Property",
> > "$test.aTestMethod()"
> > };
> >
> > /**
> > - * Test to see that "dangerous" methods are forbidden
> > + * Test to see that "dangerous" methods are forbidden
> > + * @exception Exception
> > */
> > public void testBadMethodCalls()
> > throws Exception
> > @@ -89,7 +94,8 @@
> > }
> >
> > /**
> > - * Test to see that "dangerous" methods are forbidden
> > + * Test to see that "dangerous" methods are forbidden
> > + * @exception Exception
> > */
> > public void testGoodMethodCalls()
> > throws Exception
> > @@ -109,7 +115,7 @@
> > Context c = new VelocityContext();
> > c.put("test", this);
> >
> > - try
> > + try
> > {
> > for (int i=0; i < templateStrings.length; i++)
> > {
> > @@ -117,15 +123,15 @@
> > {
> > fail ("Should have evaluated: " + templateStrings[i]);
> > }
> > -
> > +
> > if (!shouldeval && doesStringEvaluate(ve,c,templateStrings[i]))
> > {
> > fail ("Should not have evaluated: " + templateStrings[i]);
> > }
> > }
> >
> > - }
> > - catch (Exception e)
> > + }
> > + catch (Exception e)
> > {
> > fail(e.toString());
> > }
> > @@ -133,10 +139,12 @@
> >
> > private boolean doesStringEvaluate(VelocityEngine ve, Context c, String inputString) throws ParseErrorException, MethodInvocationException, ResourceNotFoundException, IOException
> > {
> > - 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.equals(inputString);
> > + return (result.length() > 0 ) && !result.equals(inputString);
> > }
> >
> > private String testProperty;
> > @@ -144,14 +152,27 @@
> > {
> > return testProperty;
> > }
> > -
> > +
> > public int aTestMethod()
> > {
> > return 1;
> > }
> > -
> > +
> > public void setProperty(String val)
> > {
> > testProperty = val;
> > }
> > +
> > +
> > + public Collection getCollection()
> > + {
> > + Collection c = new HashSet();
> > + c.add("aaa");
> > + c.add("bbb");
> > + c.add("ccc");
> > + return c;
> > + }
> > +
> > }
> > +
> > +
> >
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
--
Forio Business Simulations
Will Glass-Husain
wglass@forio.com
www.forio.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org