Re: security audit

Will Glass-Husain said:
...
>     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


*if* such a thing were implemented, then it should be done in such a
way that developers can turn it on or off in the velocity.properties.
being able to use Class, Classloader, and reflection can be very
useful in debugging templates.  also, there may be those using those
with velocity legitimately (i could envision that for code or doc
generation).  i would also say that the default setting should allow
such things.

> (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


i'm not so sure this is the right approach.  it seems to me that
developers already control what files are accessible via the resource
loader(s).  given the use-case you seem to be describing, won't you be
wanting to have different resource configs for each of your
users/template designers?  otherwise, you might cut them off from
general application templates they shouldn't be using, but they might
still use other users' templates.

ATM, i think for your case, the best approach might be to
extend/alter/replace the standard #include and #parse directives with
your own custom implementation.


> (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)

this is not really a Velocity issue.  if unsafe objects are put into
the template, that is wholly the developer's responsibility.  and
personally, your suggested solution sounds like a half-hearted hack; i
would suggest wrapping the dangerous object to expose *only* what the
designer needs.

> (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.


my response here is largely the same as for #3.   as for the
VelocityServlet, it has not been under active development for some
time now, and per brief discussion here on the list some time ago, it
is no longer the recommended servlet for velocity-based web
applications (AFAIU).  that responsibility has fallen to the servlets
provided in the velocity-tools project.  the toolbox function provided
there allows developers to "override" the request, session, response,
and application (ServletContext) objects should they so desire.  they
may override them with any object they wish, including "tools" that
wrap the the standard objects to expose only the "safe" methods.

if you really wish to continue using the VelocityServlet as your base,
extend it and override the createContext() method to keep the request
and response from being added.

Nathan Bubna
nathan@xxxxxxxx