Merge 1610491 from trunk:

SECURITY (CVE-2014-0226): Fix a race condition in scoreboard handling,
which could lead to a heap buffer overflow.  Thanks to Marek Kroemeke
working with HP's Zero Day Initiative for reporting this.

* include/scoreboard.h: Add ap_copy_scoreboard_worker.

* server/scoreboard.c (ap_copy_scoreboard_worker): New function.

* modules/generators/mod_status.c (status_handler): Use it.

Reviewed by: trawick, jorton, covener
Submitted by: jorton, trawick, covener

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@1610515 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 2d1b08a54b5810c86d7164294eca483aecfd128c..77b47617f442439ed1b6fa3b355583f8e8f31379 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.2.28
 
+  *) SECURITY: CVE-2014-0226 (cve.mitre.org)
+     Fix a race condition in scoreboard handling, which could lead to
+     a heap buffer overflow.  [Joe Orton, Eric Covener, Jeff Trawick]
+
   *) mod_cache, mod_disk_cache: With CacheLock enabled, responses with a Vary 
      header might not get the benefit of the thundering herd protection due to 
      an incorrect internal cache key.  PR 50317. 

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 4a48c3e0dbb5c43b7c4b9dd2cc496a0f44a9869e..3d2701e77bef918a4541098347419a29649f0a7b 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -151,6 +151,7 @@
  * 20051115.31 (2.2.23) Add forcerecovery to proxy_balancer_shared struct
  * 20051115.32 (2.2.24) Add ap_get_exec_line
  * 20051115.33 (2.2.24) Add ap_pregsub_ex()
+ * 20051115.34 (2.2.28) Add ap_copy_scoreboard_worker()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503232UL /* "AP22" */
@@ -158,7 +159,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20051115
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 33                    /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 34                    /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

diff --git a/include/scoreboard.h b/include/scoreboard.h
index bf43cd3fad19ec17cd3cb69673027dc85ede8553..b884073354049d29850f3265f37d3a1295cc280d 100644 (file)
--- a/include/scoreboard.h
+++ b/include/scoreboard.h
@@ -189,7 +189,24 @@ AP_DECLARE(int) ap_update_child_status_from_indexes(int child_num, int thread_nu
                                                     int status, request_rec *r);
 void ap_time_process_request(ap_sb_handle_t *sbh, int status);
 
+/** Return a pointer to the worker_score for a given child, thread pair.
+ * @param child_num The child number.
+ * @param thread_num The thread number.
+ * @return A pointer to the worker_score structure.
+ * @deprecated This function is deprecated, use ap_copy_scoreboard_worker instead. 
+ */
 AP_DECLARE(worker_score *) ap_get_scoreboard_worker(int x, int y);
+
+/** Copy the contents of a worker's scoreboard entry.  The contents of
+ * the worker_score structure are copied verbatim into the dest
+ * structure.
+ * @param dest Output parameter.
+ * @param child_num The child number.
+ * @param thread_num The thread number.
+ */
+AP_DECLARE(void) ap_copy_scoreboard_worker(worker_score *dest,
+                                           int child_num, int thread_num);
+
 AP_DECLARE(process_score *) ap_get_scoreboard_process(int x);
 AP_DECLARE(global_score *) ap_get_scoreboard_global(void);
 AP_DECLARE(lb_score *) ap_get_scoreboard_lb(int lb_num);

diff --git a/modules/generators/mod_status.c b/modules/generators/mod_status.c
index 1c64d6e2509cf6fe0bfefabe3ead2a9cfaf53482..d0d3205b8053ff9e374a7678400c509cffc4c4da 100644 (file)
--- a/modules/generators/mod_status.c
+++ b/modules/generators/mod_status.c
@@ -241,7 +241,7 @@ static int status_handler(request_rec *r)
 #endif
     int short_report;
     int no_table_report;
-    worker_score *ws_record;
+    worker_score *ws_record = apr_palloc(r->pool, sizeof *ws_record);
     process_score *ps_record;
     char *stat_buffer;
     pid_t *pid_buffer, worker_pid;
@@ -333,7 +333,7 @@ static int status_handler(request_rec *r)
         for (j = 0; j < thread_limit; ++j) {
             int indx = (i * thread_limit) + j;
 
-            ws_record = ap_get_scoreboard_worker(i, j);
+            ap_copy_scoreboard_worker(ws_record, i, j);
             res = ws_record->status;
             stat_buffer[indx] = status_flags[res];
 

diff --git a/server/scoreboard.c b/server/scoreboard.c
index 281253b844caa2cde94589e25d90ee6a22d57f81..0ff316047591f5b68e4e30d97e0c169d17ef30f0 100644 (file)
--- a/server/scoreboard.c
+++ b/server/scoreboard.c
@@ -510,6 +510,21 @@ AP_DECLARE(worker_score *) ap_get_scoreboard_worker(int x, int y)
     return &ap_scoreboard_image->servers[x][y];
 }
 
+AP_DECLARE(void) ap_copy_scoreboard_worker(worker_score *dest, 
+                                           int child_num,
+                                           int thread_num)
+{
+    worker_score *ws = ap_get_scoreboard_worker(child_num, thread_num);
+
+    memcpy(dest, ws, sizeof *ws);
+
+    /* For extra safety, NUL-terminate the strings returned, though it
+     * should be true those last bytes are always zero anyway. */
+    dest->client[sizeof(dest->client) - 1] = '\0';
+    dest->request[sizeof(dest->request) - 1] = '\0';
+    dest->vhost[sizeof(dest->vhost) - 1] = '\0';
+}
+
 AP_DECLARE(process_score *) ap_get_scoreboard_process(int x)
 {
     if ((x < 0) || (server_limit < x)) {