[PATCH] protect the environment

[Dean Gaudet <dg...@arctic.org>]

The only portable format for environment variable names matches
/^[a-zA-Z][a-zA-Z0-9_]*$/.  Enforce that by whacking anything else with
an underscore.

I was smoking crack when I said there was a security hole.  There may
still be, but I'm hard pressed to show it because I suspect it will
only exist on some unixes.  My suspicion is that you can send bogus
HTTP header fields containing random crud for the key and that random
crud will be included in an HTTP_* environment variable name.  We all
know that variable values need to be treated specially, but what about
shells that do Wrong Things with the names?

I'm proposing this for 1.3 and 1.2.  (For 1.3 I'd actually like to
improve the performance of this, but leave that for now.)

Dean

Index: main/util_script.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/util_script.c,v
retrieving revision 1.90
diff -u -r1.90 util_script.c
--- util_script.c	1998/01/11 20:55:19	1.90
+++ util_script.c	1998/01/13 21:47:33
@@ -129,11 +129,14 @@
     char *res = pstrcat(a, "HTTP_", w, NULL);
     char *cp = res;
 
-    while (*++cp)
-	if (*cp == '-')
+    while (*++cp) {
+	if (!isalnum(*cp) && *cp != '_') {
 	    *cp = '_';
-	else
+	}
+	else {
 	    *cp = toupper(*cp);
+	}
+    }
 
     return res;
 }
@@ -145,6 +148,7 @@
     char **env = (char **) palloc(p, (env_arr->nelts + 2) * sizeof(char *));
     int i, j;
     char *tz;
+    char *whack;
 
     j = 0;
     tz = getenv("TZ");
@@ -153,7 +157,18 @@
     for (i = 0; i < env_arr->nelts; ++i) {
 	if (!elts[i].key)
 	    continue;
-	env[j++] = pstrcat(p, elts[i].key, "=", elts[i].val, NULL);
+	env[j] = pstrcat(p, elts[i].key, "=", elts[i].val, NULL);
+	whack = env[j];
+	if (isdigit(*whack)) {
+	    *whack++ = '_';
+	}
+	while (*whack != '=') {
+	    if (!isalnum(*whack) && *whack != '_') {
+		*whack = '_';
+	    }
+	    ++whack;
+	}
+	++j;
     }
 
     env[j] = NULL;

Re: [PATCH] protect the environment

[Marc Slemko <ma...@worldgate.com>]

On Tue, 13 Jan 1998, Dean Gaudet wrote:

> On Tue, 13 Jan 1998, Dean Gaudet wrote:
> 
> >      tz = getenv("TZ");
> 
> Why is TZ special cased?  Isn't this a job for PassEnv?  Doesn't this make
> it impossible for users to control TZ with the documented directives? 

TZ is special cased because it has been around since before PassEnv and is
one of the few variables that "normal" environments may want to pass.  Not
that I have ever been in such a normal environment, but...

I'm not sure that changing it is a great idea...

Re: [PATCH] protect the environment

[Dean Gaudet <dg...@arctic.org>]

On Tue, 13 Jan 1998, Dean Gaudet wrote:

>      tz = getenv("TZ");

Why is TZ special cased?  Isn't this a job for PassEnv?  Doesn't this make
it impossible for users to control TZ with the documented directives? 

Dean