Everything

php

HTTP Response Splitting

php.internals

Christian Schneider

21 years ago

I looked through the bug database and the archive of this mailing list
but couldn't find any reference to HTTP Response Splitting. I apoligize
if this has been discussed before :-)

Basically it means that web applications return unfiltered user-supplied
data in the HTTP header, most commonly when doing a redirect a la
header("Location: $location");

See http://www.sanctuminc.com/pdf/Whitepaper_HTTPResponse.pdf for more
information.

Should we disallow, i.e. strip CRs and LFs from the string passed to
header() to fix the most common vulnerability in current applications?
Another idea would be to give a warning and discard the header but I
think I prefer silently stripping the characters.

Are there anything we break by doing that apart from removing the
possibility to send multiple headers with one header() call which wasn't
officially supported anyway if I'm not mistaken?

Any comments?
- Chris

Derick Rethans

21 years ago

On Mon, 11 Oct 2004, Christian Schneider wrote:

> I looked through the bug database and the archive of this mailing list
> but couldn't find any reference to HTTP Response Splitting. I apoligize
> if this has been discussed before :-)
>
> Basically it means that web applications return unfiltered user-supplied
> data in the HTTP header, most commonly when doing a redirect a la
> header("Location: $location");

This is the users' problem, not ours.

> Any comments?

Don't fix things that aren't broken. You always need to check user
supplied information.

Derick

--
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org

Christian Schneider

21 years ago

Derick Rethans wrote:

> This is the users' problem, not ours.

Sounds too much like MS to me :-)

I wrote a little patch for PHP4 which I'll deploy on our servers.

In case anyone is interested, have a look at:
http://cschneid.com/php/php4/http_reponse_splitting.patch

- Chris

Sascha Schumann

21 years ago

Considering the sapi code where each header() call lands, the
code assumes that the buffer contains exactly one HTTP
response header.  There are also some SAPI modules which
specifically expect exactly one header per call.  As such,
stripping off \n.* seems correct to me.

- Sascha

On Mon, 11 Oct 2004, Christian Schneider wrote:

Christian Schneider

21 years ago

Sascha Schumann wrote:

>     response header.  There are also some SAPI modules which
>     specifically expect exactly one header per call.  As such,
>     stripping off \n.* seems correct to me.

The HTTP standard allows header field folding (CR/LF followed by at
least one  whitespace on the next line), i.e.:
X-Some-Header: key1=value1;<CR><LF>
key2=value2;
is the same as
X-Some-Header: key1=value1; key2=value2;
and that's why my patch just replaces the CRs/LFs to spaces to unfold
the lines again. This won't break folded lines but will still defang
malicious header splitting.

- Chris

Andi Gutmans

21 years ago

I think you are right. The only problem I can see is that people added more
than one header with a header() call and it actually having worked under
some SAPIs. My guess is that this has happened quite often and it might
break quite a few apps.

Andi

At 07:36 PM 10/11/2004 +0200, Sascha Schumann wrote:

Sascha Schumann

21 years ago

On Mon, 11 Oct 2004, Andi Gutmans wrote:

> I think you are right. The only problem I can see is that people added more
> than one header with a header() call and it actually having worked under some
> SAPIs. My guess is that this has happened quite often and it might break quite
> a few apps.

In contrast to other bad programming habbits, I have not seen
this in actual code anywhere so far.

Are there any examples of real applications doing this?

We could start emitting a warning by default:

cleanup_header = false
cleanup_header_warning = true

And later change the default of cleanup_header to true.

- Sascha

Derick Rethans

21 years ago

On Tue, 12 Oct 2004, Sascha Schumann wrote:

> On Mon, 11 Oct 2004, Andi Gutmans wrote:
>
> > I think you are right. The only problem I can see is that people added more
> > than one header with a header() call and it actually having worked under some
> > SAPIs. My guess is that this has happened quite often and it might break quite
> > a few apps.
>
>     In contrast to other bad programming habbits, I have not seen
>     this in actual code anywhere so far.

I did, can't remember where it was though.

Derick

--
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org