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);
         }
     }
 

Reply via email to