All,
I recently made a change to the CSRF prevention filter in
98187ffef4cec6043a602b500a2e3dfd5ef71c20 that removed duplicate csrf
tokens in URLs in case they were being repeatedly run through
HttpServletResponse.encode().
It's possible there is a bug hiding in there but I'd like some feedback.
I'm using Velocity Tools to generate URLs and there is a tool class
called LinkTool. It generates links with a builder-style interface with
a base URL, parameters, and so on.
After upgrading to 9.0.115, I'm finding that a very small number of
these links are being broken due to an interaction between LinkTool, my
code, and the CsrfPreventionFilter's massaging of the URLs its producing.
The LinkTool is configured to produce HTML5-style URLs where the &
characters are encoded as &. I did triple-check to confirm that this
was both expected and a reasonable reading of the HTML standard, which
it appears to be. That is, use of & is actually recommended in HTML
pages. RFC 6068 and WHATWG agree on this point whi, frankly, is amazing
enough that I'll go ahead and call it a Law.
Anyway, it looks like the original URL is being produced something like
this:
/context/path?csrf=C076D4BC8A49A67BE4EE7517C3A0129D
So far so good. Note that the URL has already gone through
response.encodeURL() because it's got a csrf parameter in it.
Then another parameter is added by parsing the URL above (because
#reasons, this is where my code comes in) and the URL then becomes:
/context/path?amp;id=4&csrf=C076D4BC8A49A67BE4EE7517C3A0129D
I haven't traced through the code, but I believe what's happening is this:
1. My code, through LinkTool, adds the "id" = "4" parameter
2. LinkTool re-generates the URL String including an & parameter
separator
3. LinkTool runs the result through response.encodeURL
4. CsrfPreventionFilter removes any existing CSRF query parameters; it
finds one at the beginning of the URL and removes everything up to the
next & character
5. CsrfPreventionFilter adds its csrf token to the end of the URL
The URL is now rendered and it's broken. :/
I think plausible arguments could be made that this is a bug in any of
these 3 products: mine, VelocityTools, or Tomcat.
I'm primarily interested in getting this fixed in *my* product, but I
wonder how many other products it might effect for similar reasons.
So, the topic here is only Tomcat-related: do we think that the
CsrfPreventionFilter should consider & to be a request parameter
separator for the purposes of removing its own duplicates?
My reading of a few specs suggests that looking for "&" should be
safe since ; is a reserved character and ought to be escaped if the user
actually intends for there to be an honest-to-goodness semicolon in
their request parameter name or value. Thus, & must mean that a
literal & was intended.
So I think it's safe (and proper) for Tomcat to consume "&" and not
just "&" in this case.
Opinions?
-chris
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
