6

I've been told that using HttpServletRequest.getQueryString() in a response header makes my application susceptible to HTTP response splitting attacks, but I just don't see how.

It's clear in the case of getParameter(String), which decodes percent-encoded values, but getQueryString() does not do that. From the documentation:

The value is not decoded by the container.

Source code snippet illustrating what I'm doing:

String path = "some_url";

String qs = req.getQueryString();
if (qs != null)
    path += "?" + qs;

// response instanceof HttpServletResponse
response.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY);
response.setHeader("Location", path);

I tried to reproduce the issue, and I just get the percent-encoded newlines echoed back to me in the response. When I change the code to getParameter(…), it works as expected (except that my container is nice enough to strip the newlines from the header value, but in theory at least it works). This similar question on Stack Overflow asks the same, and a comment to the answer pointing out that getQueryString() does not decode got no response.

Am I missing something here? Or is the advice I got wrong?

  • I've been told - Do you have a reference to where you heard it? Did someone say it after seeing or hearing about your code. – Neil Smithline Nov 29 '15 at 22:49
  • @NeilSmithline A source code analysis tool identified use of getQueryString() and setHeader(…) exactly like in the sample code snippet as the (possible) vulnerability. Unfortunately I do not want to go into more details, while that report hasn't been disproved/the vulnerability hasn't been resolved, as doing so may reveal the identity of the software that got identified as having a potential vulnerability. – mwl Nov 29 '15 at 23:43
1

The general pattern of using data that comes from the user without sanitizing it is a source of vulnerabilities. Typical things like XSS and SQLi. In this case you are taking the input from the user and directly making it a header. This could lead to response splitting, header injection, and maybe another problem or two (eg: CSRF may be possible if you can inject Javascript). But you state that:

my container is nice enough to strip the newlines from the header value

so this is preventing actual exploit.

I suspect that even if you were to filter the input such as:

path += "?" + sanitize(qs);

for an appropriate sanitize function (say from OWASP's AntiSamy project) that your static analyzer will continue to give you this finding. So, in the end, you're going to be marking this finding as a false positive (Not an issue if you're using Fortify). The only question is whether you should include a code change before marking the finding as a false positive.

My take is that if the API explicitly says that it is responsible for secure handling of newlines and other special characters, then no action is needed on your part. When I look at the Javadoc for HttpServletResponse.setHeader, I see no mention of how special characters will be handled. So, unless the spec explicitly states a secure handling of this (I don't think it does but I didn't look this up in the Servlet spec), I would recommend sanitizing the input and rerunning the static scanner. If, as I expect, it continues to report this issue, then mark it a false positive and rest assured that all is well.

The reason I feel that you should only trust this behavior if it is specified in the spec is that that is the only behavior that is guaranteed. A new version of your Servlet container or another container altogether might change this behavior. It is very unlikely you would detect such a change though it would lead your site vulnerable.

| improve this answer | |
  • I'm not asking about getParameter(String), it's pretty clear how that works (I mentioned it to show that I've done some research into this topic). I'm trying to understand how getQueryString(), which does not decode the (presumable percent-)encoded input, poses a problem. Since I don't even understand why the tool complains, I'm not comfortable with believing it once it no longer complains without understanding the possible vulnerability. So the "satisfy the tool" approach you seem to recommend isn't exactly what I'm looking for here. – mwl Nov 30 '15 at 1:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.