DO NOT REPLY [Bug 10449] New: - suexec allows environment variables not in the safe list

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=10449>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=10449

suexec allows environment variables not in the safe list

           Summary: suexec allows environment variables not in the safe list
           Product: Apache httpd-1.3
           Version: 1.3.25
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: Normal
          Priority: Other
         Component: Other
        AssignedTo: bugs@httpd.apache.org
        ReportedBy: alex-news@oenone.demon.co.uk


Due to the way suexec checks environment variables in the environment against
the compiled-in safe list to decide if variables should be allowed through to
the executed CGI program, environment varibles *not* explicitly defined in the
safe list are allowed through if the beginning of the variable name matches a
variable in the safe list.  This may be by design, in which case the suexec
documentation should be amended appropriately.  I cannot see how this could ever
be a security problem.

The "problem" appears in both the 1.3.26 and the 2.0.39 versions of suexec.c,
and I have verified it at runtime in Apache 1.3.26.  The problematic code appears in
apache_1.3.26/src/support/suexec.c, function clean_env():

static void clean_env(void)
{
    char pathbuf[512];
    char **cleanenv;
    char **ep;
    int cidx = 0;
    int idx;


    if ((cleanenv = (char **) calloc(AP_ENVBUF, sizeof(char *))) == NULL) {
        log_err("emerg: failed to malloc memory for environment\n");
        exit(120);
    }

    sprintf(pathbuf, "PATH=%s", SAFE_PATH);
    cleanenv[cidx] = strdup(pathbuf);
    cidx++;

    for (ep = environ; *ep && cidx < AP_ENVBUF-1; ep++) {
        if (!strncmp(*ep, "HTTP_", 5)) {
            cleanenv[cidx] = *ep;
            cidx++;
        }
        else {
            for (idx = 0; safe_env_lst[idx]; idx++) {
                if (!strncmp(*ep, safe_env_lst[idx],
                             strlen(safe_env_lst[idx]))) {
                     ^^^^^^^^^^^^^^^^^^^^^^^^
                     ! note only checked to length safe_env_lst[idx] variable!
                    cleanenv[cidx] = *ep;
                    cidx++;
                    break;
                }
            }
        }
    }

    cleanenv[cidx] = NULL;

    environ = cleanenv;
}

I suppose one way of fixing this to do exact comparisions would be (avoiding
calling strlen() on an 'untrusted' environment variable):

            for (idx = 0; safe_env_lst[idx]; idx++) {
                if (!strncmp(*ep, safe_env_lst[idx],
                             strlen(safe_env_lst[idx])
                     && *ep[strlen(safe_env_lst[idx])]=='\0')) {
                    cleanenv[cidx] = *ep;
                    cidx++;
                    break;
                }
            }

Alternatively this could just be described as a feature, in which case this
paragraph in htdocs/manual/suexec.html.html should be amended appropriately:

      <li>
        <strong>Can we successfully clean the process environment
        to ensure safe operations?</strong> 

        <blockquote>
          suEXEC cleans the process' environment by establishing a
          safe execution PATH (defined during configuration), as
          well as only passing through those variables whose names
          are listed in the safe environment list (also created
          during configuration).
        </blockquote>
      </li>

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org