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
Index: server/util_script.c
===================================================================
--- server/util_script.c (revision 1006168)
+++ server/util_script.c (working copy)
@@ -67,11 +67,14 @@
*cp++ = '_';
while ((c = *w++) != 0) {
- if (!apr_isalnum(c)) {
+ if (apr_isalnum(c)) {
+ *cp++ = apr_toupper(c);
+ }
+ else if (c == '-') {
*cp++ = '_';
}
else {
- *cp++ = apr_toupper(c);
+ return NULL;
}
}
*cp = 0;
@@ -175,8 +178,8 @@
continue;
}
#endif
- else {
- apr_table_addn(e, http2env(r->pool, hdrs[i].key), hdrs[i].val);
+ else if ((env_temp = http2env(r->pool, hdrs[i].key)) != NULL) {
+ apr_table_addn(e, env_temp, hdrs[i].val);
}
}