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.

* modules/lua/lua_request.c (lua_ap_scoreboard_worker): Likewise.

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

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

diff --git a/CHANGES b/CHANGES
index 041c6a56561bf64880476a5f49d24cab8102f389..e3d0dd2188cefa56f88bedb4fa544fd2088d2c97 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,10 @@
 
 Changes with Apache 2.4.10
 
+  *) SECURITY: CVE-2014-0226 (cve.mitre.org)
+     Fix a race condition in scoreboard handling, which could lead to
+     a heap buffer overflow.  [Joe Orton]
+
   *) mod_ssl: Extend the scope of SSLSessionCacheTimeout to sessions
      resumed by TLS session resumption (RFC 5077). [Rainer Jung]
 

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 7156f6015e52e873386f84a1ae616209f833dfa0..a36a9997c390105b7637c06b8fae98c5daefd96d 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -431,6 +431,7 @@
  * 20120211.34 (2.4.10-dev) AP_DEFAULT_HANDLER_NAME/AP_IS_DEFAULT_HANDLER_NAME
  * 20120211.35 (2.4.10-dev) Add "r", "must_rebind", and last_backend_conn  
                             to util_ldap_connection_t
+ * 20120211.36 (2.4.10-dev) Add ap_copy_scoreboard_worker()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -438,7 +439,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20120211
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 35                   /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 36                   /* 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 2fd2960e4995073fa2be1aa5db9ec67faa1cf977..d218545a0f4eddb76abce9b7936beee3e2c38293 100644 (file)
--- a/include/scoreboard.h
+++ b/include/scoreboard.h
@@ -183,8 +183,25 @@ AP_DECLARE(int) ap_update_child_status_from_conn(ap_sb_handle_t *sbh, int status
 AP_DECLARE(void) ap_time_process_request(ap_sb_handle_t *sbh, int status);
 
 AP_DECLARE(worker_score *) ap_get_scoreboard_worker(ap_sb_handle_t *sbh);
+
+/** 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_from_indexes(int child_num,
                                                                 int thread_num);
+
+/** Copy the contents of a worker 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);
 

diff --git a/modules/generators/mod_status.c b/modules/generators/mod_status.c
index fe832b32d05d7281cd2f8f628b3abcb633d7d7d7..6bcf592b57703eba1898337a3a9a1c06dc34a5b3 100644 (file)
--- a/modules/generators/mod_status.c
+++ b/modules/generators/mod_status.c
@@ -194,7 +194,7 @@ static int status_handler(request_rec *r)
     long req_time;
     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;
@@ -306,7 +306,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_from_indexes(i, j);
+            ap_copy_scoreboard_worker(ws_record, i, j);
             res = ws_record->status;
 
             if ((i >= max_servers || j >= threads_per_child)
@@ -637,7 +637,7 @@ static int status_handler(request_rec *r)
 
         for (i = 0; i < server_limit; ++i) {
             for (j = 0; j < thread_limit; ++j) {
-                ws_record = ap_get_scoreboard_worker_from_indexes(i, j);
+                ap_copy_scoreboard_worker(ws_record, i, j);
 
                 if (ws_record->access_count == 0 &&
                     (ws_record->status == SERVER_READY ||

diff --git a/modules/lua/lua_request.c b/modules/lua/lua_request.c
index 9c399bdb0439834effee6bddebf7d20b9721e95d..8b740c227f1495d8c4463d93e958b3ff149c50fa 100644 (file)
--- a/modules/lua/lua_request.c
+++ b/modules/lua/lua_request.c
@@ -1245,16 +1245,22 @@ static int lua_ap_scoreboard_process(lua_State *L)
  */
 static int lua_ap_scoreboard_worker(lua_State *L)
 {
-    int i,
-        j;
-    worker_score   *ws_record;
+    int i, j;
+    worker_score *ws_record = NULL;
+    request_rec *r = NULL;
 
     luaL_checktype(L, 1, LUA_TUSERDATA);
     luaL_checktype(L, 2, LUA_TNUMBER);
     luaL_checktype(L, 3, LUA_TNUMBER);
+
+    r = ap_lua_check_request_rec(L, 1);
+    if (!r) return 0;
+
     i = lua_tointeger(L, 2);
     j = lua_tointeger(L, 3);
-    ws_record = ap_get_scoreboard_worker_from_indexes(i, j);
+    ws_record = apr_palloc(r->pool, sizeof *ws_record);
+
+    ap_copy_scoreboard_worker(ws_record, i, j);
     if (ws_record) {
         lua_newtable(L);
 

diff --git a/server/scoreboard.c b/server/scoreboard.c
index 24e9bc61faae9f642d2ae857041921f1adee580b..d971e9bf1c4c3843d2ae1669c6f04618af961a2e 100644 (file)
--- a/server/scoreboard.c
+++ b/server/scoreboard.c
@@ -579,6 +579,21 @@ AP_DECLARE(worker_score *) ap_get_scoreboard_worker(ap_sb_handle_t *sbh)
                                                  sbh->thread_num);
 }
 
+AP_DECLARE(void) ap_copy_scoreboard_worker(worker_score *dest, 
+                                           int child_num,
+                                           int thread_num)
+{
+    worker_score *ws = ap_get_scoreboard_worker_from_indexes(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) || (x >= server_limit)) {