You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@velocity.apache.org by Christopher Schultz <ch...@christopherschultz.net> on 2012/01/05 22:05:28 UTC
Can clients really specify 'layout' parameter in the URL?
All,
I was doing some testing in a webapp that uses Velocity 1.7 and Velocity
Tools 2.0 (plus a few as-yet-unreleased patches) and I'm using
VelocityLayoutServlet.
I found some errors in my log file about a particular 'layout' not being
found. Coincidentally, I had a request parameter called "layout" with
some data in it and it seemed to be triggering a change to the layout
file that VelocityViewServlet attempts to use.
Seeing an opportunity, I tried this URL:
http://localhost:8217/webapp/random.do?layout=../WEB-INF/web.xml
Guess what happened? web.xml was dumped to my browser.
I'm doing some additional investigation as to why this request parameter
is being set in the Velocity Context, but it does not appear that my
page itself is doing it.
Has anyone seen anything like this before?
-chris
Re: Can clients really specify 'layout' parameter in the URL?
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Nathan,
On 1/9/12 11:30 PM, Nathan Bubna wrote:
> i'm just trying to figure out what your
> tools.view.servlet.layout.directory setting is.
It's the default, which is "/layout" relative to the web app root.
>> Perhaps this is only a problem if the webapp.resource.loader.path=/, but
>> I believe that is the default configuration.
>
> The point is merely that if the servlet is picking templates via url
> path, how is that any more secure than picking a layout template via a
> param.
I don't allow remote clients to request .vm files directly: all .vm
requests must come from within the webapp. In this case,
VelocityLayoutServlet will happily return the contents of any file in
the webapp if specified in the "layout" parameter of any request that
ultimately ends up serving-up a .vm file.
That means that I can request "foo.action?layout=../WEB-INF/web.xml" and
get my web.xml returned to a remote client. There's nothing I can do
about it other than patch VelocityLayoutServlet (which is what I've
done) to avoid allowing that to happen.
I can't tell my servlet container not to respond to *.xml requests (and
that's a slippery slope, anyway, since I'd have to list every file type
I never intend to return to a remote client) for obvious reasons.
There are no protections whatsoever on what VelocityLayoutServlet will
load as a layout. I think the "layout" parameter ought to be restricted
to the tools.view.servlet.layout.directory so that ".." will not work.
Presumably, everything in the "layouts" directory is intended to be used
as a layout. My deployment descriptor is not.
Also, I have mapped *.vm to VelocityLayoutServlet. The layout parameter
is not sanitized against any kind of URL pattern -- it just blindly
loads the file as a "template" and anything that doesn't cause an error
when calling ServletContext.getInputStream (or whatever) will succeed.
> Both are meant to be constrained to particular directories and
> not allow general file access.
Agreed.
> Those constraints ought to be coming from the resource loader
> [configuration], not the servlet.
The container is responsible for preventing remote users from requesting
resources that are not appropriate -- like WEB-INF/web.xml. It cannot
prevent servlets from reading and serving-up any file they want.
It might be able to prevent a servlet from reading from /etc/passwd
(because it's using the application's resource loader) but any file
within the web application is fair game. Think of all the standard
configuration files that contain database credentials, etc. that are
available to unauthenticated remote users in this way?
How do you suppose the servlet container could prevent a servlet from
loading WEB-INF/web.xml?
-chris
Re: Can clients really specify 'layout' parameter in the URL?
Posted by Nathan Bubna <nb...@gmail.com>.
On Mon, Jan 9, 2012 at 11:00 AM, Christopher Schultz
<ch...@christopherschultz.net> wrote:
> Nathan,
>
> On 1/8/12 2:04 PM, Nathan Bubna wrote:
>> The tools.view.servlet.layout.directory property is fairly important
>> here, as it is prefixed to layout paths. i didn't think just slipping
>> .. into the path would get Velocity to load things out of that
>> directory. Seems like this isn't anymore risky than allowing third
>> parties to use #parse, #inclue or anything else that loads/renders
>> templates dynamically.
>
> That's true, but if you never code your templates to use request
> /parameters/ as part of a path to #parse or #include, then you are safe.
> In this case, the request parameter is used if it's there by
> VelocityLayoutServlet: you can't prevent a remote user from selecting
> the template because it's part of the servlet.
>
>> So, before we jump to calling this an inherent security flaw. What is
>> your layout directory?
>
> What's *in* it? I have about 10 templates (header.vm, footer.vm,
> Default.vm, etc.) in there.
is !- in it
i'm just trying to figure out what your
tools.view.servlet.layout.directory setting is.
>> What is your resource loader configuration?
>
> All defaults in development (where it's reproducible), and these
> settings in prod (which is also vulnerable):
>
> resource.loader=webapp
> webapp.resource.loader.class=org.apache.velocity.tools.view.servlet.WebappLoader
> webapp.resource.loader.cache=true
> webapp.resource.loader.path=/
>
> (I think those are roughly the defaults, anyway).
>
>> The resource loaders really ought not be able to load any file either,
>> otherwise the VelocityViewServlet itself becomes a risk, right?
>
> Perhaps this is only a problem if the webapp.resource.loader.path=/, but
> I believe that is the default configuration.
The point is merely that if the servlet is picking templates via url
path, how is that any more secure than picking a layout template via a
param. Both are meant to be constrained to particular directories and
not allow general file access. Those constraints ought to be coming
from the resource loader [configuration], not the servlet.
> It's fairly easy to try on your own webapp.at
not at the moment, tomorrow, maybe.
>
> -chris
>
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
For additional commands, e-mail: user-help@velocity.apache.org
Re: Can clients really specify 'layout' parameter in the URL?
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Nathan,
On 1/8/12 2:04 PM, Nathan Bubna wrote:
> The tools.view.servlet.layout.directory property is fairly important
> here, as it is prefixed to layout paths. i didn't think just slipping
> .. into the path would get Velocity to load things out of that
> directory. Seems like this isn't anymore risky than allowing third
> parties to use #parse, #inclue or anything else that loads/renders
> templates dynamically.
That's true, but if you never code your templates to use request
/parameters/ as part of a path to #parse or #include, then you are safe.
In this case, the request parameter is used if it's there by
VelocityLayoutServlet: you can't prevent a remote user from selecting
the template because it's part of the servlet.
> So, before we jump to calling this an inherent security flaw. What is
> your layout directory?
What's *in* it? I have about 10 templates (header.vm, footer.vm,
Default.vm, etc.) in there.
> What is your resource loader configuration?
All defaults in development (where it's reproducible), and these
settings in prod (which is also vulnerable):
resource.loader=webapp
webapp.resource.loader.class=org.apache.velocity.tools.view.servlet.WebappLoader
webapp.resource.loader.cache=true
webapp.resource.loader.path=/
(I think those are roughly the defaults, anyway).
> The resource loaders really ought not be able to load any file either,
> otherwise the VelocityViewServlet itself becomes a risk, right?
Perhaps this is only a problem if the webapp.resource.loader.path=/, but
I believe that is the default configuration.
It's fairly easy to try on your own webapp.
-chris
Re: Can clients really specify 'layout' parameter in the URL?
Posted by Nathan Bubna <nb...@gmail.com>.
The tools.view.servlet.layout.directory property is fairly important
here, as it is prefixed to layout paths. i didn't think just slipping
.. into the path would get Velocity to load things out of that
directory. Seems like this isn't anymore risky than allowing third
parties to use #parse, #inclue or anything else that loads/renders
templates dynamically.
So, before we jump to calling this an inherent security flaw. What is
your layout directory? What is your resource loader configuration?
The resource loaders really ought not be able to load any file either,
otherwise the VelocityViewServlet itself becomes a risk, right?
On Thu, Jan 5, 2012 at 1:22 PM, Alex Fedotov <al...@kayak.com> wrote:
> I think there is some kind of fall-back sequence coded in one of the
> Velocity context implementations where if a key is not found in the context
> then it is also looked up as a request parameter, session attribute, etc.
> If I remember correctly it is in the ViewToolContext class.
>
> On Thu, Jan 5, 2012 at 4:15 PM, Christopher Schultz <
> chris@christopherschultz.net> wrote:
>
>> All,
>>
>> On 1/5/12 4:05 PM, Christopher Schultz wrote:
>> > I found some errors in my log file about a particular 'layout' not being
>> > found. Coincidentally, I had a request parameter called "layout" with
>> > some data in it and it seemed to be triggering a change to the layout
>> > file that VelocityViewServlet attempts to use.
>>
>> I am using a custom subclass of VelocityViewServlet that changes the
>> error handling and also the Context creation by taking the user's Struts
>> locale and putting it into the Context. Here's the method:
>>
>> protected Context createContext(HttpServletRequest request,
>> HttpServletResponse response)
>> {
>> Context ctx = super.createContext(request, response);
>>
>> // Don't clobber an existing key
>> if(!ctx.containsKey("locale"))
>> {
>> Locale locale = null;
>> HttpSession session = request.getSession(false);
>> if(null != session)
>> locale = (Locale)session.getAttribute(Globals.LOCALE_KEY);
>>
>> if(null == locale)
>> locale = request.getLocale();
>>
>> ctx.put("locale", locale);
>> }
>>
>> // DEBUG
>> System.err.println("createContext: 'layout'="
>> + ctx.get("layout"));
>> ctx.put("theContext", ctx);
>> return ctx;
>> }
>>
>> I added that debugging code at the bottom, and the log confirms that, at
>> this stage, 'layout' is null in the context. I stuck the 'theContext'
>> into itself so I could inspect it from the page, like this:
>>
>> request: $!request.getParameter('layout')
>> context: $!theContext.get('layout')
>> app: $!request.servletContext.getAttribute('layout')
>> bare layout = $!layout
>>
>> The output from this template displays the following:
>>
>> request: Help.vm
>> context: Help.vm
>> app:
>> bare layout: Help.vm
>>
>> So, somehow the request parameter is in fact being copied into the
>> velocity context, where it affects VelocityLayoutServlet's rendering of
>> the page.
>>
>> I'll get a stack trace of where the context key is being set, next.
>>
>> -chris
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
For additional commands, e-mail: user-help@velocity.apache.org
Re: Can clients really specify 'layout' parameter in the URL?
Posted by Alex Fedotov <al...@kayak.com>.
I think there is some kind of fall-back sequence coded in one of the
Velocity context implementations where if a key is not found in the context
then it is also looked up as a request parameter, session attribute, etc.
If I remember correctly it is in the ViewToolContext class.
On Thu, Jan 5, 2012 at 4:15 PM, Christopher Schultz <
chris@christopherschultz.net> wrote:
> All,
>
> On 1/5/12 4:05 PM, Christopher Schultz wrote:
> > I found some errors in my log file about a particular 'layout' not being
> > found. Coincidentally, I had a request parameter called "layout" with
> > some data in it and it seemed to be triggering a change to the layout
> > file that VelocityViewServlet attempts to use.
>
> I am using a custom subclass of VelocityViewServlet that changes the
> error handling and also the Context creation by taking the user's Struts
> locale and putting it into the Context. Here's the method:
>
> protected Context createContext(HttpServletRequest request,
> HttpServletResponse response)
> {
> Context ctx = super.createContext(request, response);
>
> // Don't clobber an existing key
> if(!ctx.containsKey("locale"))
> {
> Locale locale = null;
> HttpSession session = request.getSession(false);
> if(null != session)
> locale = (Locale)session.getAttribute(Globals.LOCALE_KEY);
>
> if(null == locale)
> locale = request.getLocale();
>
> ctx.put("locale", locale);
> }
>
> // DEBUG
> System.err.println("createContext: 'layout'="
> + ctx.get("layout"));
> ctx.put("theContext", ctx);
> return ctx;
> }
>
> I added that debugging code at the bottom, and the log confirms that, at
> this stage, 'layout' is null in the context. I stuck the 'theContext'
> into itself so I could inspect it from the page, like this:
>
> request: $!request.getParameter('layout')
> context: $!theContext.get('layout')
> app: $!request.servletContext.getAttribute('layout')
> bare layout = $!layout
>
> The output from this template displays the following:
>
> request: Help.vm
> context: Help.vm
> app:
> bare layout: Help.vm
>
> So, somehow the request parameter is in fact being copied into the
> velocity context, where it affects VelocityLayoutServlet's rendering of
> the page.
>
> I'll get a stack trace of where the context key is being set, next.
>
> -chris
>
>
Re: Can clients really specify 'layout' parameter in the URL?
Posted by Christopher Schultz <ch...@christopherschultz.net>.
All,
On 1/5/12 4:05 PM, Christopher Schultz wrote:
> I found some errors in my log file about a particular 'layout' not being
> found. Coincidentally, I had a request parameter called "layout" with
> some data in it and it seemed to be triggering a change to the layout
> file that VelocityViewServlet attempts to use.
I am using a custom subclass of VelocityViewServlet that changes the
error handling and also the Context creation by taking the user's Struts
locale and putting it into the Context. Here's the method:
protected Context createContext(HttpServletRequest request,
HttpServletResponse response)
{
Context ctx = super.createContext(request, response);
// Don't clobber an existing key
if(!ctx.containsKey("locale"))
{
Locale locale = null;
HttpSession session = request.getSession(false);
if(null != session)
locale = (Locale)session.getAttribute(Globals.LOCALE_KEY);
if(null == locale)
locale = request.getLocale();
ctx.put("locale", locale);
}
// DEBUG
System.err.println("createContext: 'layout'="
+ ctx.get("layout"));
ctx.put("theContext", ctx);
return ctx;
}
I added that debugging code at the bottom, and the log confirms that, at
this stage, 'layout' is null in the context. I stuck the 'theContext'
into itself so I could inspect it from the page, like this:
request: $!request.getParameter('layout')
context: $!theContext.get('layout')
app: $!request.servletContext.getAttribute('layout')
bare layout = $!layout
The output from this template displays the following:
request: Help.vm
context: Help.vm
app:
bare layout: Help.vm
So, somehow the request parameter is in fact being copied into the
velocity context, where it affects VelocityLayoutServlet's rendering of
the page.
I'll get a stack trace of where the context key is being set, next.
-chris