Awful lot of formatting changes mixed in here, makes it hard to tell what changed. :(
On 2/18/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> 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:[EMAIL PROTECTED]">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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
