You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Christopher Schultz <ch...@christopherschultz.net> on 2011/04/01 02:21:18 UTC

Re: [OT] Protecting against HTTP response splitting

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ronald,

On 3/31/2011 7:05 AM, Ronald Klop wrote:
> I would say that some proper input validation solves your problem.
> Does new URL(redirectURL).toString() give an exception on invalid url's?

new URL(String) will throw a MalformedURLException if there are illegal
characters in the URL.

I suppose that's good enough for my purposes: the only returnURLs that
should be generated should be coming from our own application, and if
they are broken, it's a bug. If a MalformedURLException is thrown, it
should be due to some sort of malicious use and the user is better off
getting a nasty error than just about anything else.

Thanks,
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2VGn4ACgkQ9CaO5/Lv0PBk5gCdF5DMiC7/BrXTxDHayWzChU9W
Dc8AoKq1E+6Y2NVTbTuS0vn1NtMhzo0C
=2Kss
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Konstantin,

On 4/20/2011 11:56 AM, Konstantin Kolinko wrote:
> 2011/4/20 Christopher Schultz <ch...@christopherschultz.net>:
>>
>> I was considering scouring the URL/URI specs for exactly what characters
>> are allowed but then decided that I didn't really care: I was mostly
>> concerned with thwarting a response-splitting attack and avoiding \r and
>> \n does that.
> 
> See HTTP spec on what is allowed in headers.

It's in the RFC 822, ARPA Internet Text Messages, not the HTTP spec.

>> This isn't intended to be an outgoing HTTP header value validator.
>>
>> Technically, this is over-engineered because it looks for /either/ \r
>> /or/ \n, rather than \r\n which should be the only way to exploit such a
>> vulnerability. :)
> 
> You are wrong. This way is not the only one.

I suppose my implementation may be both simplistic and draconian. For
example, a header value such as "This isCRLF my value" is perfectly
valid and does not represent a response-splitting attack, though it
would trigger an error from my filter. Also, header values are allowed
to contain CRs and LFs, just not next to each other. I'm okay with
violating both of these provisions since my application doesn't make use
of them.

I'm interested in where you see another opportunity to split an HTTP
response using HTTP headers.

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2wNrcACgkQ9CaO5/Lv0PBblgCePNngKWEhiCemVvnDMj+TR1WN
1tkAnRroXgx6KFVIkyEY2DkbaSX/DecT
=0RHt
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/4/20 Christopher Schultz <ch...@christopherschultz.net>:
>
> I was considering scouring the URL/URI specs for exactly what characters
> are allowed but then decided that I didn't really care: I was mostly
> concerned with thwarting a response-splitting attack and avoiding \r and
> \n does that.

See HTTP spec on what is allowed in headers.

>
> This isn't intended to be an outgoing HTTP header value validator.
>
> Technically, this is over-engineered because it looks for /either/ \r
> /or/ \n, rather than \r\n which should be the only way to exploit such a
> vulnerability. :)
>

You are wrong. This way is not the only one.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Konstantin,

On 4/19/2011 4:37 AM, Konstantin Kolinko wrote:
> 2011/4/19 Christopher Schultz <ch...@christopherschultz.net>:
>>
>> Looks like I must override sendRedirect because otherwise the setHeader
>> call implemented in Response.sendRedirect isn't intercepted by the
>> wrapper class.
>>
>> For those interested, see below for the implementation I came up with.
>>
> 
>>            if(containsCRorLF(value))
>>                throw new IllegalArgumentException("Header value must
>> not contain CR or LF characters");
> 
> It would be better to check that all characters are correct ones rather
> than check for two specific incorrect characters.
> 
> Checking for \r \n only might be not enough. Though that depends on
> where the value comes from.

I was considering scouring the URL/URI specs for exactly what characters
are allowed but then decided that I didn't really care: I was mostly
concerned with thwarting a response-splitting attack and avoiding \r and
\n does that.

This isn't intended to be an outgoing HTTP header value validator.

Technically, this is over-engineered because it looks for /either/ \r
/or/ \n, rather than \r\n which should be the only way to exploit such a
vulnerability. :)

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2u6U0ACgkQ9CaO5/Lv0PCvdACgjm/Q/3IrBC318Bb0wi+WDjee
v78AoLjj9uj6mDiRWik8WV/3pQWqDXiB
=IgDT
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/4/19 Christopher Schultz <ch...@christopherschultz.net>:
>
> Looks like I must override sendRedirect because otherwise the setHeader
> call implemented in Response.sendRedirect isn't intercepted by the
> wrapper class.
>
> For those interested, see below for the implementation I came up with.
>

>            if(containsCRorLF(value))
>                throw new IllegalArgumentException("Header value must
> not contain CR or LF characters");

It would be better to check that all characters are correct ones rather
than check for two specific incorrect characters.

Checking for \r \n only might be not enough. Though that depends on
where the value comes from.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

All,

On 4/18/2011 9:51 PM, Christopher Schultz wrote:
> I'm leaning more towards just protecting against control characters in a
> header: there's no need to do a complete URL-parse to check for response
> splitting.
> 
> A simple filter that wraps the response and overrides either
> sendRedirect or setHeader(String, String) should do it.
> 
> I'd have to check to see how the two interact... whether calling
> sendRedirect on a wrapped response will also set the header on the
> wrapped response or set the header at a higher level where the wrapper
> won't get called.

Looks like I must override sendRedirect because otherwise the setHeader
call implemented in Response.sendRedirect isn't intercepted by the
wrapper class.

For those interested, see below for the implementation I came up with.

Enjoy,
- -chris

import java.io.IOException;
import java.net.MalformedURLException;

import javax.servlet.Filter;
import javax.servlet.FilterConfig;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;

/**
 * Prevents the application from setting HTTP headers that include
 * CR or LF characters. Specifically throws a MalformedURLException
 * for the sendRedirect method which is already declared to throw
 * IOException.
 */
public class HttpResponseSplittingPreventionFilter
    implements Filter
{
    public void init(FilterConfig config)
    {
    }

    /**
     * Performs the filtering operation provided by this filter.
     *
     * @param request The request being made to the server.
     * @param response The response object prepared for the client.
     * @param chain The chain of filters providing request services.
     */
    public void doFilter(ServletRequest request,
                         ServletResponse response,
                         FilterChain chain)
        throws IOException, ServletException
    {
        if(response instanceof HttpServletResponse)
            response = new Wrapper((HttpServletResponse)response);

        chain.doFilter(request, response);
    }

    /**
     * Called by the servlet container to indicate that a filter is being
     * taken out of service.<p>
     */
    public void destroy()
    {
    }

    class Wrapper
        extends HttpServletResponseWrapper
    {
        Wrapper(HttpServletResponse response)
        {
            super(response);
        }

        public void sendRedirect(String location)
            throws IOException
        {
            if(containsCRorLF(location))
                throw new MalformedURLException("CR or LF detected in
redirect URL: possible http response splitting attack");

            super.sendRedirect(location);
        }

        public void setHeader(String name, String value)
        {
            if(containsCRorLF(value))
                throw new IllegalArgumentException("Header value must
not contain CR or LF characters");

            super.setHeader(name, value);
        }

        public void addHeader(String name, String value)
        {
            if(containsCRorLF(value))
                throw new IllegalArgumentException("Header value must
not contain CR or LF characters");

            super.addHeader(name, value);
        }

        private boolean containsCRorLF(String s)
        {
            if(null == s) return false;

            int length = s.length();

            for(int i=0; i<length; ++i)
            {
                char c = s.charAt(i);

                if('\n' == c
                   || '\r' == c)
                    return true;
            }

            return false;
        }
    }
}
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2s66gACgkQ9CaO5/Lv0PDcSgCfbJnJkhqAYeZ/i7TgdGqX9adr
BZ4An1G8gf8EAdV6ywyo0b7c6PWqg+AD
=9kiL
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sebb,

Just saw your response from a few weeks back... (and responded directly
instead of to the list.. it's been a long day).

On 4/1/2011 6:16 PM, sebb wrote:
> I may be missing something here, but can't you use the ctor:
> 
> URL(URL context, String spec)
> 
> and pass in a dummy context with a suitable protocol?

Maybe. The URL may or may not be fully-qualified, relative, etc.

I'm leaning more towards just protecting against control characters in a
header: there's no need to do a complete URL-parse to check for response
splitting.

A simple filter that wraps the response and overrides either
sendRedirect or setHeader(String, String) should do it.

I'd have to check to see how the two interact... whether calling
sendRedirect on a wrapped response will also set the header on the
wrapped response or set the header at a higher level where the wrapper
won't get called.

I'll post whatever I come up with.

Thanks,
- -chris


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2s6o8ACgkQ9CaO5/Lv0PDikgCgtGkHVIGl1mJwIAXBiQ4V0qq8
auUAoIoIrsaH8LHn+U/pEVbFQK09y71D
=AMLs
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by sebb <se...@gmail.com>.
On 1 April 2011 15:49, Christopher Schultz <ch...@christopherschultz.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Ronald,
>
> On 3/31/2011 8:21 PM, Christopher Schultz wrote:
>> On 3/31/2011 7:05 AM, Ronald Klop wrote:
>>> I would say that some proper input validation solves your problem.
>>> Does new URL(redirectURL).toString() give an exception on invalid url's?
>>
>> new URL(String) will throw a MalformedURLException if there are illegal
>> characters in the URL.
>>
>> I suppose that's good enough for my purposes: the only returnURLs that
>> should be generated should be coming from our own application, and if
>> they are broken, it's a bug. If a MalformedURLException is thrown, it
>> should be due to some sort of malicious use and the user is better off
>> getting a nasty error than just about anything else.
>
> Apparently, it's more complicated than that... at least when it comes to
> my particular application... we want to keep the URLs as short as
> possible, they they are not fully-qualified in most cases. Instead, they
> are webapp-relative and blindly passing them into the java.net.URL
> constructor fails even for "real" URLs because they have no protocol.

I may be missing something here, but can't you use the ctor:

URL(URL context, String spec)

and pass in a dummy context with a suitable protocol?

> Now, I could add code to fully-qualify them, but then I'd be doing work
> I'm already asking the container to do for me (since
> HttpServletResponse.sendRedirect is required to fully-qualify the URL
> anyway) and I'd prefer to rely on the container for that task -- it's
> likely to do a better job, anyway :)
>
> I think I'm doing to standardize on simply scanning for troublesome
> characters like \r and \n and throwing a MalformedURLException or
> something like that.
>
> If anyone else has any good ideas or Warnings about what might be a
> naive sanitization check, I'd be glad to hear them.
>
> Thanks,
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (MingW32)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk2V5gsACgkQ9CaO5/Lv0PBgfwCeOrioFeSvp8iUJ51a9qJqAny3
> 8QkAn0c12aRinn7eoGUoAgA2uYydVQA/
> =bwLF
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/4/1 Christopher Schultz <ch...@christopherschultz.net>:
> I think I'm doing to standardize on simply scanning for troublesome
> characters like \r and \n and throwing a MalformedURLException or
> something like that.

You'd better scan for allowed characters. The \r and \n are not the
only ones where the things may go wrong.

> If anyone else has any good ideas or Warnings about what might be a
> naive sanitization check, I'd be glad to hear them.
>

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ronald,

On 3/31/2011 8:21 PM, Christopher Schultz wrote:
> On 3/31/2011 7:05 AM, Ronald Klop wrote:
>> I would say that some proper input validation solves your problem.
>> Does new URL(redirectURL).toString() give an exception on invalid url's?
> 
> new URL(String) will throw a MalformedURLException if there are illegal
> characters in the URL.
> 
> I suppose that's good enough for my purposes: the only returnURLs that
> should be generated should be coming from our own application, and if
> they are broken, it's a bug. If a MalformedURLException is thrown, it
> should be due to some sort of malicious use and the user is better off
> getting a nasty error than just about anything else.

Apparently, it's more complicated than that... at least when it comes to
my particular application... we want to keep the URLs as short as
possible, they they are not fully-qualified in most cases. Instead, they
are webapp-relative and blindly passing them into the java.net.URL
constructor fails even for "real" URLs because they have no protocol.

Now, I could add code to fully-qualify them, but then I'd be doing work
I'm already asking the container to do for me (since
HttpServletResponse.sendRedirect is required to fully-qualify the URL
anyway) and I'd prefer to rely on the container for that task -- it's
likely to do a better job, anyway :)

I think I'm doing to standardize on simply scanning for troublesome
characters like \r and \n and throwing a MalformedURLException or
something like that.

If anyone else has any good ideas or Warnings about what might be a
naive sanitization check, I'd be glad to hear them.

Thanks,
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2V5gsACgkQ9CaO5/Lv0PBgfwCeOrioFeSvp8iUJ51a9qJqAny3
8QkAn0c12aRinn7eoGUoAgA2uYydVQA/
=bwLF
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Leon,

On 4/1/2011 1:49 AM, Leon Rosenberg wrote:
> On Fri, Apr 1, 2011 at 2:21 AM, Christopher Schultz
> <ch...@christopherschultz.net> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Ronald,
>>
>> On 3/31/2011 7:05 AM, Ronald Klop wrote:
>>> I would say that some proper input validation solves your problem.
>>> Does new URL(redirectURL).toString() give an exception on invalid url's?
>>
>> new URL(String) will throw a MalformedURLException if there are illegal
>> characters in the URL.
>>
> 
> This will work for 'correct urls', however, you don't necessary need
> to send correct urls, and I suppose you don't want to:
> Consider this, struts1 like action:
> 	public ActionForward execute(ActionMapping mapping, FormBean bean,
> HttpServletRequest req, HttpServletResponse res) throws Exception {
> 
> 		//do something.... useful
> 		res.sendRedirect("pageResult?page=1");
> 		return null;
> 	}
> 
> This is not a syntactically correct url, but it will work in all
> browsers and save you a lot of stress in multi-url (i18n) portals.
> I would solve your problem by having multiple entry points for the
> actions which than can specify the final redirect path.

Yeah, I was thinking about this, too... instead of passing the actual
URL, just have a list of predefined URLs like "home" or "display" or
whatever and then just pass-around a symbolic name... that way, the
worst a malicious user can do is get the wrong name and go to a
different part of the webapp... instead of being able to redirect to
evilsite.com.

That requires more work, of course, and may be the ultimate solution we
choose... but it's not going to work for some particular actions because
they really can be redirected to an arbitrary location within our
webapp, and enumerating those would not really be possible.

Thanks,
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2V5sAACgkQ9CaO5/Lv0PA7cgCglfyxvxL2wzNeTJIOiWsmrCqA
CV4AoLgdmc3bG5I19J2tf9BLDxXme1Sh
=iQAo
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: [OT] Protecting against HTTP response splitting

Posted by Leon Rosenberg <ro...@gmail.com>.
On Fri, Apr 1, 2011 at 2:21 AM, Christopher Schultz
<ch...@christopherschultz.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Ronald,
>
> On 3/31/2011 7:05 AM, Ronald Klop wrote:
>> I would say that some proper input validation solves your problem.
>> Does new URL(redirectURL).toString() give an exception on invalid url's?
>
> new URL(String) will throw a MalformedURLException if there are illegal
> characters in the URL.
>

This will work for 'correct urls', however, you don't necessary need
to send correct urls, and I suppose you don't want to:
Consider this, struts1 like action:
	public ActionForward execute(ActionMapping mapping, FormBean bean,
HttpServletRequest req, HttpServletResponse res) throws Exception {

		//do something.... useful
		res.sendRedirect("pageResult?page=1");
		return null;
	}

This is not a syntactically correct url, but it will work in all
browsers and save you a lot of stress in multi-url (i18n) portals.
I would solve your problem by having multiple entry points for the
actions which than can specify the final redirect path.

regards
Leon

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org