---------- Forwarded message ----------
From: Tommy <tommyc_at_google.com>
Date: Fri, Aug 3, 2012 at 2:06 PM
Subject: Re: [jsr340-experts] Re: [servlet-spec users] Re: Re: security
concern (protocol parameter parsing order)
To: Mark Thomas <markt_at_apache.org>
Cc: jsr340-experts_at_servlet-spec.java.net
[please forward to the rest of the list on my behalf]
On Fri, Jul 27, 2012 at 6:03 PM, Mark Thomas <markt_at_apache.org> wrote:
> > ---------- Forwarded message ----------
> > From: Tommy <tommyc_at_google.com>
> > Date: Thu, Jul 19, 2012 at 8:15 PM
> > Subject: Fwd: [servlet-spec users] [jsr340-experts] Re: security concern
> > (protocol parameter parsing order)
> > To: Wenbo Zhu <wenboz_at_google.com>
>
> > Note that the vulnerability is not limited to cases where developers fail
> > to specify a form action; it's also a problem when they are
> > indiscriminately forwarding query parameters or when a javascript XHR
> posts
> > to document.location. That seems like a lot of things that every
> developer
> > has to be on the lookout for, especially when it could be fixed without
> any
> > intervention on their part.
>
> How would existing applications be fixed without any intervention?
> Present parameters from the query string after those in the request body?
>
Yes, that's my suggestion to fix the problem with minimal interference.
>
> > At the most basic level, it feels broken that it's not possible to
> > distinguish a user's form input from query string input using the
> > HttpServletRequest#getParameter API. Since there are a number of avenues
> > for the attack, and it's really easy for folks to get wrong if they're
> not
> > actively considering it, it seems to me like an API change would be
> > appropriate.
>
> Changing the API is a lit easier than getting folks to use the new API -
> especially as the old one will have to continue to work to provide
> backwards compatibility.
>
Agreed. But I think it's valuable to start the process of deprecating the
ambiguous accessor. Maintaining support for #getParameter just involves
mashing the more specific methods' return values together, which seems
straightforward.
public String getParameter(String name) {
String value = getBodyParameter(name);
return (value != null) ? value : getQueryParameter(name);
}
>
> > I like the idea of being able to opt out of the fixed behavior for folks
> > who may break, since people who are ambivalent will benefit and those who
> > are broken will realize it and work to fix it.
>
> I'm not sure where this opt-out could be implemented. Maybe
> getParameter(String name, boolean bodyFirst) with getParameter(String
> name) delegating to getParameter(name, true).
>
There was a suggestion in previous responses to control the behavior in a
configuration file for the server. One could also imagine a system
property that could toggle it. Both of these changes would not affect the
actual calling code for accessing parameters.
>
> > In the long run I'd prefer to see getQueryParameter/getBodyParameter
> > introduced and getParameter deprecated to eliminate the confusion. It'd
> > also be good to raise awareness on the issue, since the only
> documentation
> > of this behavior that I found was in the servlet spec (rather than, say,
> > javadoc on HttpServletReqeust).
>
> There are lots of possibilities for API changes, complicated by the need
> to minimize breakage for existing users and to retain backwards
> compatibility. My preference is for something along the lines of the
> above that 'just works' for most folks and has an easy work-around for
> everyone else.
>
I think that the suggestion for fixing #getParameter behavior to order body
parameters first (with some configuration option to force old behavior)
passes the backwards compatibility and "just works" tests. Going forward,
though, I really think there should be distinct methods for accessing
parameters to eliminate any confusion.
>
> Mark
>
>