Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (nike.apache.org: domain of ejacobs@bluehost.com designates
 67.20.126.230 as permitted sender)
Message-ID: <5090AD37.1070303@bluehost.com>
Date: Tue, 30 Oct 2012 22:46:47 -0600
From: Eric Jacobs <ejacobs@bluehost.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:16.0) Gecko/20121016 Thunderbird/16.0.1
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: [patch] Fix cross-user symlink race condition vulnerability
Content-Type: multipart/mixed;
 boundary="------------030300030502080609050702"

This is a multi-part message in MIME format.
--------------030300030502080609050702
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

There is a race condition vulnerability in httpd 2.2.23 (also present in 
previous releases) that allows a malicious user to serve arbitrary files 
from nearly anywhere on a server that isn't protected by strict os level 
permissions. In a shared hosting environment, this is a big vulnerability.

If you would like more information on the exploit itself, please let me 
know. I have a proof of concept that is able to hit the exploit with 
100% success.

This is my first patch submitted to Apache, so I'm sorry if I've missed 
something. I'm aware that this doesn't meet some of the code standards 
that are in place (e.g, it doesn't work at all on Windows), but I wanted 
to put it out there anyway.

The patch that fixes the vulnerability is attached. Thank you in advance 
for the feedback.

-- 

Eric Jacobs
Junior Systems Administrator
Bluehost.com

--------------030300030502080609050702
Content-Type: text/x-patch;
 name="httpd-2.2.23-symlink-protection.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="httpd-2.2.23-symlink-protection.patch"

diff -rupN httpd-2.2.23-orig/modules/mappers/mod_userdir.c httpd-2.2.23/modules/mappers/mod_userdir.c
--- httpd-2.2.23-orig/modules/mappers/mod_userdir.c     2011-02-07 19:58:51.000000000 -0700
+++ httpd-2.2.23/modules/mappers/mod_userdir.c  2012-09-18 21:55:11.297206652 -0600
@@ -51,6 +51,7 @@
 
 #include "apr_strings.h"
 #include "apr_user.h"
+#include "apr_env.h"
 
 #define APR_WANT_STRFUNC
 #include "apr_want.h"
@@ -63,6 +64,7 @@
 #include "httpd.h"
 #include "http_config.h"
 #include "http_request.h"
+#include "http_log.h"
 
 #if !defined(WIN32) && !defined(OS2) && !defined(BEOS) && !defined(NETWARE)
 #define HAVE_UNIX_SUEXEC
@@ -313,7 +315,7 @@ static int translate_userdir(request_rec
          * used, for example, to run a CGI script for the user.
          */
         if (filename && (!*userdirs
-                      || ((rv = apr_stat(&statbuf, filename, APR_FINFO_MIN,
+                      || ((rv = apr_stat(&statbuf, filename, APR_FINFO_NORM,
                                          r->pool)) == APR_SUCCESS
                                              || rv == APR_INCOMPLETE))) {
             r->filename = apr_pstrcat(r->pool, filename, dname, NULL);
@@ -324,6 +326,11 @@ static int translate_userdir(request_rec
             if (*userdirs && dname[0] == 0)
                 r->finfo = statbuf;
 
+            /* This is used later on to make sure the symlink exploit is not
+             * exploitable.
+             */
+            apr_table_set(r->subprocess_env, "SPT_DOCROOT", filename);
+
             /* For use in the get_suexec_identity phase */
             apr_table_setn(r->notes, "mod_userdir_user", w);
 
diff -rupN httpd-2.2.23-orig/server/core.c httpd-2.2.23/server/core.c
--- httpd-2.2.23-orig/server/core.c     2011-09-08 09:59:38.000000000 -0600
+++ httpd-2.2.23/server/core.c  2012-09-25 16:04:14.863185577 -0600
@@ -21,6 +21,7 @@
 #include "apr_hash.h"
 #include "apr_thread_proc.h"    /* for RLIMIT stuff */
 #include "apr_hooks.h"
+#include "apr_env.h"            /* for symlink protection + userdir stuff */
 
 #define APR_WANT_IOVEC
 #define APR_WANT_STRFUNC
@@ -3676,6 +3677,9 @@ static int default_handler(request_rec *
     int errstatus;
     apr_file_t *fd = NULL;
     apr_status_t status;
+    core_server_config *csconf;
+    apr_finfo_t post_open_dirstat;
+    apr_finfo_t post_open_finfo;
     /* XXX if/when somebody writes a content-md5 filter we either need to
      *     remove this support or coordinate when to use the filter vs.
      *     when to use this code
@@ -3687,6 +3691,12 @@ static int default_handler(request_rec *
 
     d = (core_dir_config *)ap_get_module_config(r->per_dir_config,
                                                 &core_module);
+
+    /* Pull in the vhost's server configuration. Mostly we just use this to
+     * check the document root user against the open file user.
+     */
+    csconf = ap_get_module_config(r->server->module_config, &core_module);
+
     bld_content_md5 = (d->content_md5 & 1)
                       && r->output_filters->frec->ftype != AP_FTYPE_RESOURCE;
 
@@ -3760,6 +3770,66 @@ static int default_handler(request_rec *
             return HTTP_FORBIDDEN;
         }
 
+        /* This is where the magic is. If a user is trying to hit the apache
+         * symlink race condition, then we will know about it here.
+         */
+
+        char *sp_docroot = apr_table_get(r->subprocess_env, "SPT_DOCROOT");
+        apr_status_t post_dirstat_rv;
+        apr_status_t post_fdstat_rv;
+
+        if (strcmp(csconf->ap_document_root, "/usr/local/apache/htdocs") == 0
+            && sp_docroot != NULL){
+            /* Then this is a request coming in from mod_userdir.c, and we
+             * need to stat what we stored in sp-docroot, instead of
+             * ap_document_root
+             */
+
+            post_dirstat_rv = apr_stat(&post_open_dirstat, sp_docroot,
+            APR_FINFO_USER | APR_FINFO_LINK, r->pool);
+
+        } else {
+            /* Then this a request that matched a vhost, so ap_document_root
+             * will be what we are looking for.
+             */
+            post_dirstat_rv = apr_stat(&post_open_dirstat,
+            csconf->ap_document_root, APR_FINFO_USER | APR_FINFO_LINK, r->pool);
+        }
+        
+        post_fdstat_rv = apr_stat_fd(&post_open_finfo, fd, APR_FINFO_USER,
+        r->pool);
+
+        if (((post_dirstat_rv != APR_SUCCESS && post_dirstat_rv != APR_INCOMPLETE)
+            || !(post_open_dirstat.valid & APR_FINFO_USER))
+            || ((post_fdstat_rv != APR_SUCCESS && post_fdstat_rv != APR_INCOMPLETE)
+            || !(post_open_finfo.valid & APR_FINFO_USER))) {
+            /* Then we couldn't stat either the directory root of the vhost
+             * (very unlikely) or we couldn't stat the open file descriptor
+             * (probably impossible).
+             */
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
+            "Could not stat directory root or open file. Aborting request.");
+            apr_file_close(fd);
+            return HTTP_NOT_FOUND;
+        }
+
+
+        if (apr_uid_compare(r->finfo.user, post_open_dirstat.user)
+            != APR_SUCCESS || apr_uid_compare(post_open_finfo.user, r->finfo.user)
+            != APR_SUCCESS) {
+            /* Then we've caught a race condition abuser. */
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
+            "Caught race condition abuser. attacker: %i, victim: %i"
+            " open file owner: %i, open file: %s", post_open_dirstat.user, r->finfo.user,
+            post_open_finfo.user, r->filename);
+
+            apr_file_close(fd);
+            /* Return 404 because we don't want an attacker to be able to test
+             * what files are where based on the return of an error.
+             */
+            return HTTP_NOT_FOUND;
+        }
+
         ap_update_mtime(r, r->finfo.mtime);
         ap_set_last_modified(r);
         ap_set_etag(r);
diff -rupN httpd-2.2.23-orig/srclib/apr/file_io/unix/filestat.c httpd-2.2.23/srclib/apr/file_io/unix/filestat.c
--- httpd-2.2.23-orig/srclib/apr/file_io/unix/filestat.c        2007-10-16 21:35:55.000000000 -0600
+++ httpd-2.2.23/srclib/apr/file_io/unix/filestat.c     2012-09-14 20:28:41.169804347 -0600
@@ -336,4 +336,51 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
     }
 }
 
+APR_DECLARE(apr_status_t) apr_stat_fd(apr_finfo_t *finfo, apr_file_t *fd,
+                                   apr_int32_t wanted, apr_pool_t *pool)
+{
+    struct_stat info;
+    int srv = fstat(fd->filedes, &info);
 
+    if (srv == 0) {
+        finfo->pool = pool;
+        finfo->fname = fd->fname;
+        fill_out_finfo(finfo, &info, wanted);
+        if (wanted & APR_FINFO_LINK)
+            wanted &= ~APR_FINFO_LINK;
+        return (wanted & ~finfo->valid) ? APR_INCOMPLETE : APR_SUCCESS;
+    }
+    else {
+#if !defined(ENOENT) || !defined(ENOTDIR)
+#error ENOENT || ENOTDIR not defined; please see the
+#error comments at this line in the source for a workaround.
+        /*
+         * If ENOENT || ENOTDIR is not defined in one of the your OS's
+         * include files, APR cannot report a good reason why the stat()
+         * of the file failed; there are cases where it can fail even though
+         * the file exists.  This opens holes in Apache, for example, because
+         * it becomes possible for someone to get a directory listing of a 
+         * directory even though there is an index (eg. index.html) file in 
+         * it.  If you do not have a problem with this, delete the above 
+         * #error lines and start the compile again.  If you need to do this,
+         * please submit a bug report to http://www.apache.org/bug_report.html
+         * letting us know that you needed to do this.  Please be sure to 
+         * include the operating system you are using.
+         */
+        /* WARNING: All errors will be handled as not found
+         */
+#if !defined(ENOENT) 
+        return APR_ENOENT;
+#else
+        /* WARNING: All errors but not found will be handled as not directory
+         */
+        if (errno != ENOENT)
+            return APR_ENOENT;
+        else
+            return errno;
+#endif
+#else /* All was defined well, report the usual: */
+        return errno;
+#endif
+    }
+}

--------------030300030502080609050702--