Re: security audit

I have a similar situation where I can't trust users, or rather would 
prefer not to have to worry about what they're doing, so I appreciate 
you highlighting these issues.  Here is what I'm doing and some thoughts 
about what I'll do as a result of this thread:

Will Glass-Husain wrote:
> (1) Template writer can execute any method from any class (such as Runtime.exit 
> or File.delete) by getting the classloader and instantiating a class.
> http://nagoya.apache.org/eyebrowse/ReadMsg?listId=102&msgNo=5980 
>
>      Solution-- patch Velocity to prevent execution of methods of 
from classes Class, Classloader, and java.lang.reflect.*
> 
http://nagoya.apache.org/eyebrowse/ReadMsg?listName=velocity-dev@xxxxxxxxxxxxxxxxxx&msgNo=7802

My solution is to have the templates load via a restricted ClassLoader. 
 This is easy enough to create, I can have the ClassLoader wrap the 
existing ClassLoader, but then when requesting a given class, I can 
refuse all but a handful of allowed classes.

Your patch might be useful if there are classes I need embedded objects 
to use but in a controlled fashion.  It'd be nice if the SecurityManager 
could be used, but it's per JVM instead of classloader or thread.

> (2) Template writer can use #include and #parse to load any file in template 
> path
>
>      Solution - patch to Velocity allowing the developer to control what files 
> are accessible to input directives
>      
> http://nagoya.apache.org/eyebrowse/ReadMsg?listName=velocity-dev@xxxxxxxxxxxxxxxxxx&msgNo=7781

We have layed custom resource loaders, so this isn't an issue for us. 
It's all virtual and they have no way to get out of what I'm giving them.

> (3) If Torque objects are available in the context, the writer can execute 
> arbitrary SQL code with
>      $TorqueObject.Peer.executeStatement("delete from big_table")
>
>      Solution - override getPeer method in Torque object to return null.  (note 
> - must also do solution # 1 or template writer can acquire an instance of the 
> superclass)

I don't offer objects this dangerous.  I've been thinking about giving 
access to a database, but it would be with a very carefully exposed 
object instance.

> (4) Generalization of #3.  Other objects in the context might have methods with 
> unwanted consequences.  Special caution for objects that are implementations of 
> interfaces -- there might be hidden methods in the implementing class that are 
> undesirable.
>
>     Solution - Review every object placed in the context.  Remove "request" and 
> "response" which are placed in the VelocityServlet context by default.

Seems like just best practice.  I think the idea of that security 
document so others using Velocity in this way could benefit from this 
thoughtful review and organize the feedback collected.

--
Serge Knystautas
President
Lokitech >> software . strategy . design >> http://www.lokitech.com
p. 301.656.5501
e. sergek@xxxxxxxxxxxx