Add alternative fixes for CVE-2007-3304:

* configure.in: Check for getpgid.

* include/mpm_common.h (ap_mpm_safe_kill): New prototype.

* server/mpm_common.c (reclaim_one_pid): Ensure pid validity before
calling apr_proc_wait().
(ap_mpm_safe_kill): New function.

* server/mpm/prefork/prefork.c, server/mpm/worker/worker.c,
server/mpm/experimental/event/event.c: Use ap_mpm_safe_kill() on pids
from the scoreboard, throughout.

* include/ap_mmn.h: Minor bump.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@551843 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 471ca759c887c92249bbc0bf11ec2e62d0387186..9bae28e1d27dc6d4e5b19f2bce38151bb53c5528 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,10 @@
 Changes with Apache 2.3.0
   [Remove entries to the current 2.0 and 2.2 section below, when backported]
 
+  *) SECURITY: CVE-2007-3304 (cve.mitre.org)
+     prefork, worker, event MPMs: Ensure that the parent process cannot
+     be forced to kill processes outside its process group.  [Joe Orton]
+
   *) SECURITY: CVE-2006-5752 (cve.mitre.org)
      mod_status: Fix a possible XSS attack against a site with a public
      server-status page and ExtendedStatus enabled, for browsers which

diff --git a/configure.in b/configure.in
index e906f681038145e98e056baf50b53f0707f6eb09..dd560e1ecf89297386cf0eb163b1a4ad90fce771 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -399,6 +399,7 @@ initgroups \
 bindprocessor \
 prctl \
 timegm \
+getpgid
 )
 
 dnl confirm that a void pointer is large enough to store a long integer

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 206a8339294601698375c6897b0d2031678426eb..888147a23cdf3e947906b77938e6d410bd856e2b 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -129,6 +129,7 @@
  *                         ap_parse_mutex()
  * 20060905.3 (2.3.0-dev)  Added conn_rec::clogging_input_filters.
  * 20060905.4 (2.3.0-dev)  Added proxy_balancer::sticky_path.
+ * 20060905.5 (2.3.0-dev)  Added ap_mpm_safe_kill()
  *
  */
 
@@ -137,7 +138,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20060905
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 4                    /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 5                    /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

diff --git a/include/mpm_common.h b/include/mpm_common.h
index 20a74ac6424d04c9f9d8d8597f09147765226703..30786751c99e032789175fab9bcf8670b106ec23 100644 (file)
--- a/include/mpm_common.h
+++ b/include/mpm_common.h
@@ -144,6 +144,19 @@ void ap_register_extra_mpm_process(pid_t pid);
 int ap_unregister_extra_mpm_process(pid_t pid);
 #endif
 
+/**
+ * Safely signal an MPM child process, if the process is in the
+ * current process group.  Otherwise fail.
+ * @param pid the process id of a child process to signal
+ * @param sig the signal number to send
+ * @return APR_SUCCESS if signal is sent, otherwise an error as per kill(3);
+ * APR_EINVAL is returned if passed either an invalid (< 1) pid, or if
+ * the pid is not in the current process group
+ */
+#ifdef AP_MPM_WANT_RECLAIM_CHILD_PROCESSES
+apr_status_t ap_mpm_safe_kill(pid_t pid, int sig);
+#endif
+
 /**
  * Determine if any child process has died.  If no child process died, then
  * this process sleeps for the amount of time specified by the MPM defined

diff --git a/server/mpm/experimental/event/event.c b/server/mpm/experimental/event/event.c
index 9d242b3867e6f84aec135fc1dce57e576673b302..d0128e5101cc01d270e0502ea1d61e63cea43701 100644 (file)
--- a/server/mpm/experimental/event/event.c
+++ b/server/mpm/experimental/event/event.c
@@ -2070,12 +2070,10 @@ int ap_mpm_run(apr_pool_t * _pconf, apr_pool_t * plog, server_rec * s)
 
             active_children = 0;
             for (index = 0; index < ap_daemons_limit; ++index) {
-                if (MPM_CHILD_PID(index) != 0) {
-                    if (kill(MPM_CHILD_PID(index), 0) == 0) {
-                            active_children = 1;
-                            /* Having just one child is enough to stay around */
-                            break;
-                    }
+                if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) {
+                    active_children = 1;
+                    /* Having just one child is enough to stay around */
+                    break;
                 }
             }
         } while (!shutdown_pending && active_children &&

diff --git a/server/mpm/prefork/prefork.c b/server/mpm/prefork/prefork.c
index 5f0b1fbd0dab74dc9afb4bc1f5412f737cd59f46..98b07e6e768b14482d48a0151d23bcef752f6991 100644 (file)
--- a/server/mpm/prefork/prefork.c
+++ b/server/mpm/prefork/prefork.c
@@ -1096,7 +1096,7 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
         for (index = 0; index < ap_daemons_limit; ++index) {
             if (ap_scoreboard_image->servers[index][0].status != SERVER_DEAD) {
                 /* Ask each child to close its listeners. */
-                kill(MPM_CHILD_PID(index), AP_SIG_GRACEFUL);
+                ap_mpm_safe_kill(MPM_CHILD_PID(index), AP_SIG_GRACEFUL);
                 active_children++;
             }
         }
@@ -1134,12 +1134,10 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
 
             active_children = 0;
             for (index = 0; index < ap_daemons_limit; ++index) {
-                if (MPM_CHILD_PID(index) != 0) {
-                    if (kill(MPM_CHILD_PID(index), 0) == 0) {
-                            active_children = 1;
-                            /* Having just one child is enough to stay around */
-                            break;
-                    }
+                if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) {
+                    active_children = 1;
+                    /* Having just one child is enough to stay around */
+                    break;
                 }
             }
         } while (!shutdown_pending && active_children &&
@@ -1191,7 +1189,7 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
                  * piped loggers, etc. They almost certainly won't handle
                  * it gracefully.
                  */
-                kill(ap_scoreboard_image->parent[index].pid, AP_SIG_GRACEFUL);
+                ap_mpm_safe_kill(ap_scoreboard_image->parent[index].pid, AP_SIG_GRACEFUL);
             }
         }
     }

diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index e8eca8c197ad5391d12d3a6e01f71f8461c623a8..5b531c6ede4b805c72e04825399859539ab4f14c 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -1815,12 +1815,10 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
 
             active_children = 0;
             for (index = 0; index < ap_daemons_limit; ++index) {
-                if (MPM_CHILD_PID(index) != 0) {
-                    if (kill(MPM_CHILD_PID(index), 0) == 0) {
-                            active_children = 1;
-                            /* Having just one child is enough to stay around */
-                            break;
-                    }
+                if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) {
+                    active_children = 1;
+                    /* Having just one child is enough to stay around */
+                    break;
                 }
             }
         } while (!shutdown_pending && active_children &&

diff --git a/server/mpm_common.c b/server/mpm_common.c
index 6e485c70c21582bb17cd5d87464db9a2f5baa839..8ead7e39dde97822859215ad0f042c3fa8cce563 100644 (file)
--- a/server/mpm_common.c
+++ b/server/mpm_common.c
@@ -127,6 +127,11 @@ static int reclaim_one_pid(pid_t pid, action_t action)
     apr_proc_t proc;
     apr_status_t waitret;
 
+    /* Ensure pid sanity. */
+    if (pid < 1) {
+        return 1;
+    }        
+
     proc.pid = pid;
     waitret = apr_proc_wait(&proc, NULL, NULL, APR_NOWAIT);
     if (waitret != APR_CHILD_NOTDONE) {
@@ -306,6 +311,66 @@ void ap_relieve_child_processes(void)
         cur_extra = next;
     }
 }
+
+/* Before sending the signal to the pid this function verifies that
+ * the pid is a member of the current process group; either using
+ * apr_proc_wait(), where waitpid() guarantees to fail for non-child
+ * processes; or by using getpgid() directly, if available. */
+apr_status_t ap_mpm_safe_kill(pid_t pid, int sig)
+{
+#ifndef HAVE_GETPGID
+    apr_proc_t proc;
+    apr_status_t rv;
+    apr_exit_why_e why;
+    int status;
+
+    /* Ensure pid sanity */
+    if (pid < 1) {
+        return APR_EINVAL;
+    }
+
+    proc.pid = pid;
+    rv = apr_proc_wait(&proc, &status, &why, APR_NOWAIT);
+    if (rv == APR_CHILD_DONE) {
+#ifdef AP_MPM_WANT_PROCESS_CHILD_STATUS
+        /* The child already died - log the termination status if
+         * necessary: */
+        ap_process_child_status(&proc, why, status);
+#endif
+        return APR_EINVAL;
+    }
+    else if (rv != APR_CHILD_NOTDONE) {
+        /* The child is already dead and reaped, or was a bogus pid -
+         * log this either way. */
+        ap_log_error(APLOG_MARK, APLOG_NOTICE, rv, ap_server_conf,
+                     "cannot send signal %d to pid %ld (non-child or "
+                     "already dead)", sig, (long)pid);
+        return APR_EINVAL;
+    }
+#else
+    int pg;
+
+    /* Ensure pid sanity. */
+    if (pid < 1) {
+        return APR_EINVAL;
+    }
+
+    pg = getpgid(pid);    
+    if (pg == -1) {
+        /* Process already dead... */
+        return errno;
+    }
+
+    if (pg != getpgrp()) {
+        ap_log_error(APLOG_MARK, APLOG_ALERT, 0, ap_server_conf,
+                     "refusing to send signal %d to pid %ld outside "
+                     "process group", sig, (long)pid);
+        return APR_EINVAL;
+    }
+#endif        
+
+    return kill(pid, sig) ? errno : APR_SUCCESS;
+}
 #endif /* AP_MPM_WANT_RECLAIM_CHILD_PROCESSES */
 
 #ifdef AP_MPM_WANT_WAIT_OR_TIMEOUT