You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Malte S. Stretz" <ms...@apache.org> on 2010/10/12 16:30:18 UTC
[PATCH] mod_cgi: Mitigating some header injections by dropping invalid headers?
Hi folks,
a few days ago on a bored afternoon thumbed through the Unusual Web Bugs
presentation [1] from 24C3. On slide 19/20 the author shows a way to
inject otherwise filtered headers from Flash into CGI scripts. This is
caused by sloppy filtering on the client side and the simple translation
to environment variables (essentially y/a-z/A-Z/;s/[^A-Z]/_/g) on the
server side. That way you can set eg. the HTTP_USER_AGENT environment
variable by sending a "User!Agent:foo" header.
I had a quick look at the Apache source and the solution was simple: Just
drop headers which contain any character outside the range [a-zA-Z0-9-].
The patch against trunk is attached.
Now, my next task was to imagine any way how this could be used for XSS
attacks as claimed in the presentation. You'd need a header which
contains a dash to do so; the only relevant one I could think of was X-
Requested-With but maybe there are others I don't know of. So, is this
really needed? Dunno.
On the other hand, would it hurt to be a little less forgiving when
parsing headers? I mean, this is the 21st century, HTTP is around for
almost 20 years, by now everybody who has to write a client should know
how to format headers. RFC3875 section 4.1.18 doesn't complain either.
Cheers,
Malte
P.S.: I couldn't find anything like apr_pfree to get rid of the memory
allocated for bad headers, but if I grokked the APR docs correctly, we've
got to wait until the pool is emptied to reclaim our memory, right?
Shouldn't hurt to keep those few unused bytes around then.
[1]http://events.ccc.de/congress/2007/Fahrplan/events/2212.en.html
Re: [PATCH] mod_cgi: Mitigating some header injections by dropping invalid headers?
Posted by "Malte S. Stretz" <ms...@apache.org>.
On Tuesday 28 December 2010 15:56:15 Stefan Fritsch wrote:
> On Wednesday 08 December 2010, Malte S. Stretz wrote:
> > Hmm, no reply yet, are there any objections/comments/questions
> > about the patches? If not, anybody with enough karma to
> > commit? Just asking :)
>
> I have commited the variant without separate config directive as
> r1053357, r1053363, r1053365. Thank you very much for your patch and
> your patience.
Thanks. I almost forgot about it by now :)
Cheers,
Malte
--
Re: [PATCH] mod_cgi: Mitigating some header injections by dropping invalid headers?
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Wednesday 08 December 2010, Malte S. Stretz wrote:
> Hmm, no reply yet, are there any objections/comments/questions
> about the patches? If not, anybody with enough karma to
> commit? Just asking :)
I have commited the variant without separate config directive as
r1053357, r1053363, r1053365. Thank you very much for your patch and
your patience.
Re: [PATCH] mod_cgi: Mitigating some header injections by dropping invalid headers?
Posted by "Malte S. Stretz" <ms...@apache.org>.
On Monday 22 November 2010 23:25:06 I wrote:
> On Monday 18 October 2010 12:28:12 Malte S. Stretz wrote:
> > On Tuesday 12 October 2010 19:49:02 Malte S. Stretz wrote:
> > > On Tuesday 12 October 2010 18:13:46 William A. Rowe Jr. wrote:
> > > > On 10/12/2010 10:06 AM, Dirk-Willem van Gulik wrote:
> > > > > On 12 Oct 2010, at 15:30, Malte S. Stretz wrote:
> > > > >> I had a quick look at the Apache source and the solution was
> > > > >> simple: Just drop headers which contain any character
> > > > >> outside the range [a-zA-Z0-9-]. The patch against trunk is
> > > > >> attached.
> > > > >
> > > > > This made me think of something we had a while ago; and after
> > > > > checking the logs - big +1 from me!
> > > > [...]
>
> Time flies by... :)
>
> As it seems like an option is preferred to a workaround, here's are a
> bunch of patches. The first implements an option (an environment
> variable map-invalid-headers) to switch on the backwards
> compatibility. It got delayed because I didn't write any
> documentation yet. I'll do so if it gets accepted :) [...]
Hmm, no reply yet, are there any objections/comments/questions about the
patches? If not, anybody with enough karma to commit? Just asking :)
Cheers,
Malte
--
Re: [PATCH] mod_cgi: Mitigating some header injections by dropping invalid headers?
Posted by "Malte S. Stretz" <ms...@apache.org>.
On Monday 18 October 2010 12:28:12 Malte S. Stretz wrote:
> On Tuesday 12 October 2010 19:49:02 Malte S. Stretz wrote:
> > On Tuesday 12 October 2010 18:13:46 William A. Rowe Jr. wrote:
> > > On 10/12/2010 10:06 AM, Dirk-Willem van Gulik wrote:
> > > > On 12 Oct 2010, at 15:30, Malte S. Stretz wrote:
> > > >> I had a quick look at the Apache source and the solution was
> > > >> simple: Just drop headers which contain any character outside the
> > > >> range [a-zA-Z0-9-]. The patch against trunk is attached.
> > > >
> > > > This made me think of something we had a while ago; and after
> > > > checking the logs - big +1 from me!
> > >
> > > Agreed, with a caviat... we aught to be able to toggle this for the
> > > rare but significant legacy app that requires it... which implies a
> > > per-dir flag that can override just one CGI script out of an entire
> > > server.
> >
> > I think an option is not needed as there is a workaround. Eg. to
> > make an Accept_Encoding header work:
> >
> > SetEnvIfNoCase ^Accept.Encoding$ ^(.*)$ fix_header=$1
> > RequestHeader set Accept-Encoding %{fix_header}e env=fix_header
> >
> >[...]
>
> Attached is an updated patch which also updates the docs. It also
> includes the commit message I tried to commit it with (didn't realize
> that there are per-project commit flags).
>
> Is the documented workaround good enough or should something like an
> map- broken-headers environment variable be introduced?
Time flies by... :)
As it seems like an option is preferred to a workaround, here's are a
bunch of patches. The first implements an option (an environment variable
map-invalid-headers) to switch on the backwards compatibility. It got
delayed because I didn't write any documentation yet. I'll do so if it
gets accepted :)
The second patch transforms the #ifdef check against Auth headers into an
option, too (option map-authorization-headers). If it is accepted, I'll
write some documentation, highlighting why it is a bad idea, too. I added
it because I think an option is better than an obscure #ifdef which opens
up a rarely tested code path.
And finally, the third patch is a bit bigger, it contains a bunch of
refactoring, moving some stuff to helper routines, getting rid of unneeded
variables (which probably would have been optimized away anyway) and
adding some comments. It is probably easier to follow if you have a look
at the single commits at Github:
https://github.com/mss/httpd/compare/more-env-work
If you prefer the old way (a documented workaround instead of an option),
I rebased that code against trunk (two days old), so it applies again:
https://github.com/mss/httpd/compare/strict-env-work
Cheers,
Malte
Re: [PATCH] mod_cgi: Mitigating some header injections by dropping invalid headers?
Posted by "Malte S. Stretz" <ms...@apache.org>.
On Tuesday 12 October 2010 19:49:02 Malte S. Stretz wrote:
> On Tuesday 12 October 2010 18:13:46 William A. Rowe Jr. wrote:
> > On 10/12/2010 10:06 AM, Dirk-Willem van Gulik wrote:
> > > On 12 Oct 2010, at 15:30, Malte S. Stretz wrote:
> > >> I had a quick look at the Apache source and the solution was
> > >> simple:
> > >> Just drop headers which contain any character outside the range
> > >>
> > >> [a-zA-Z0-9-]. The patch against trunk is attached.
> > >
> > > This made me think of something we had a while ago; and after
> > > checking the logs - big +1 from me!
> >
> > Agreed, with a caviat... we aught to be able to toggle this for the
> > rare but significant legacy app that requires it... which implies a
> > per-dir flag that can override just one CGI script out of an entire
> > server.
>
> I think an option is not needed as there is a workaround. Eg. to make
> an Accept_Encoding header work:
>
> SetEnvIfNoCase ^Accept.Encoding$ ^(.*)$ fix_header=$1
> RequestHeader set Accept-Encoding %{fix_header}e env=fix_header
>[...]
Attached is an updated patch which also updates the docs. It also
includes the commit message I tried to commit it with (didn't realize that
there are per-project commit flags).
Is the documented workaround good enough or should something like an map-
broken-headers environment variable be introduced?
Cheers,
Malte
Re: [PATCH] mod_cgi: Mitigating some header injections by dropping invalid headers?
Posted by "Malte S. Stretz" <ms...@apache.org>.
On Tuesday 12 October 2010 18:13:46 William A. Rowe Jr. wrote:
> On 10/12/2010 10:06 AM, Dirk-Willem van Gulik wrote:
> > On 12 Oct 2010, at 15:30, Malte S. Stretz wrote:
> >> I had a quick look at the Apache source and the solution was simple:
> >> Just drop headers which contain any character outside the range
> >> [a-zA-Z0-9-]. The patch against trunk is attached.
> >
> > This made me think of something we had a while ago; and after
> > checking the logs - big +1 from me!
>
> Agreed, with a caviat... we aught to be able to toggle this for the
> rare but significant legacy app that requires it... which implies a
> per-dir flag that can override just one CGI script out of an entire
> server.
I think an option is not needed as there is a workaround. Eg. to make an
Accept_Encoding header work:
SetEnvIfNoCase ^Accept.Encoding$ ^(.*)$ fix_header=$1
RequestHeader set Accept-Encoding %{fix_header}e env=fix_header
(I had to use a regexp in SetEnvIf since for some reason comparing to an
invalid header doesn't work.)
Cheers,
Malte
Re: [PATCH] mod_cgi: Mitigating some header injections by dropping
invalid headers?
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 10/12/2010 10:06 AM, Dirk-Willem van Gulik wrote:
>
> On 12 Oct 2010, at 15:30, Malte S. Stretz wrote:
>
>> I had a quick look at the Apache source and the solution was simple: Just
>> drop headers which contain any character outside the range [a-zA-Z0-9-].
>> The patch against trunk is attached.
>
> This made me think of something we had a while ago; and after checking the logs - big +1 from me!
Agreed, with a caviat... we aught to be able to toggle this for the rare but
significant legacy app that requires it... which implies a per-dir flag that
can override just one CGI script out of an entire server.
Re: [PATCH] mod_cgi: Mitigating some header injections by dropping invalid headers?
Posted by Dirk-Willem van Gulik <di...@webweaving.org>.
On 12 Oct 2010, at 15:30, Malte S. Stretz wrote:
> I had a quick look at the Apache source and the solution was simple: Just
> drop headers which contain any character outside the range [a-zA-Z0-9-].
> The patch against trunk is attached.
This made me think of something we had a while ago; and after checking the logs - big +1 from me!
Dw.