From: Joe Orton Date: Tue, 17 Jul 2007 14:48:25 +0000 (+0000) Subject: Merge r551843, r551889 from trunk: X-Git-Tag: 2.2.5~156 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e56a5ff187317a055d38b63f5f76706f8907e18e;p=thirdparty%2Fapache%2Fhttpd.git Merge r551843, r551889 from trunk: 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. * server/mpm_common.c: getpgid() returns a pid_t Submitted by: jorton, jim Reviewed by: jorton, jim, rpluem git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@556936 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index fd9542fb109..6a4b87bab56 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,11 @@ Changes with Apache 2.2.5 Cache-Control header without any value. [Niklas Edmundsson ] + *) 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, Jim Jagielski] + *) mod_cache: Do not set Date or Expires when they are missing from the original response or are invalid. [Justin Erenkrantz] diff --git a/configure.in b/configure.in index 869ebd81cf0..5ab7133c22e 100644 --- a/configure.in +++ b/configure.in @@ -392,6 +392,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 9d8d89cd2c3..95871f1b4b3 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -113,6 +113,8 @@ * 20051115.3 (2.2.3) Added server_scheme member to server_rec (minor) * 20051115.4 (2.2.4) Added ap_get_server_banner() and * ap_get_server_description() (minor) + * 20051115.5 (2.2.5) Added ap_mpm_safe_kill() (minor) + * */ #define MODULE_MAGIC_COOKIE 0x41503232UL /* "AP22" */ @@ -120,7 +122,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20051115 #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 bc4480a792d..bacf76f256b 100644 --- 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 e105c25e97d..2a6913b3c3d 100644 --- a/server/mpm/experimental/event/event.c +++ b/server/mpm/experimental/event/event.c @@ -1998,12 +1998,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 ba50581b6a7..d3c93a63f68 100644 --- a/server/mpm/prefork/prefork.c +++ b/server/mpm/prefork/prefork.c @@ -1127,7 +1127,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++; } } @@ -1165,12 +1165,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 && @@ -1222,7 +1220,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 a9d822f5411..b6b9447b70e 100644 --- a/server/mpm/worker/worker.c +++ b/server/mpm/worker/worker.c @@ -1813,12 +1813,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 c654cbfb221..f80637330f3 100644 --- a/server/mpm_common.c +++ b/server/mpm_common.c @@ -126,6 +126,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) { @@ -305,6 +310,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 + pid_t 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