From: Joe Orton Date: Mon, 14 Jul 2014 19:55:04 +0000 (+0000) Subject: Merge 1610491 from trunk: X-Git-Tag: 2.4.10~23 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4bbf0508ea33b0a295f49e11810f6c6d13ba7b47;p=apache 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 041c6a5656..e3d0dd2188 100644 --- 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 7156f6015e..a36a9997c3 100644 --- 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 2fd2960e49..d218545a0f 100644 --- 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 fe832b32d0..6bcf592b57 100644 --- 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 9c399bdb04..8b740c227f 100644 --- 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 24e9bc61fa..d971e9bf1c 100644 --- 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)) {