From: Jim Jagielski Date: Wed, 7 Dec 2016 12:57:08 +0000 (+0000) Subject: Merge r1772919 from trunk: X-Git-Tag: 2.4.24~60 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d049e3ce42b89ba66c17b0cd8c4c5992ec2b12fe;p=thirdparty%2Fapache%2Fhttpd.git Merge r1772919 from trunk: mod_auth_digest: fix segfaults during shared memory exhaustion The apr_rmm_addr_get/apr_rmm_malloc() combination did not correctly check for a malloc failure, leading to crashes when we ran out of the limited space provided by AuthDigestShmemSize. This patch replaces all these calls with a helper function that performs this check. Additionally, fix a NULL-check bug during entry garbage collection. Submitted by: jchampion Reviewed/backported by: jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1773069 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index a40867accc4..db0d12b7da6 100644 --- a/CHANGES +++ b/CHANGES @@ -12,6 +12,11 @@ Changes with Apache 2.4.24 core: Mitigate [f]cgi "httpoxy" issues. [Dominic Scheirlinck , Yann Ylavic] + *) SECURITY: CVE-2016-2161 (cve.mitre.org) + mod_auth_digest: Prevent segfaults during client entry allocation when the + shared memory space is exhausted. [Maksim Malyutin , + Eric Covener, Jacob Champion] + *) SECURITY: CVE-2016-0736 (cve.mitre.org) mod_session_crypto: Authenticate the session data/cookie with a MAC (SipHash) to prevent deciphering or tampering with a padding diff --git a/STATUS b/STATUS index 134f90539ae..35076ae2052 100644 --- a/STATUS +++ b/STATUS @@ -117,13 +117,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - *) SECURITY: CVE-2016-2161 (cve.mitre.org) - mod_auth_digest: Prevent segfaults during client entry allocation when the - shared memory space is exhausted. [Maksim Malyutin , - Eric Covener, Jacob Champion] - trunk patch: http://svn.apache.org/r1772919 - 2.4.x patch: trunk works (modulo CHANGES) - +1: jchampion, jim, covener PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ New proposals should be added at the end of the list ] diff --git a/modules/aaa/mod_auth_digest.c b/modules/aaa/mod_auth_digest.c index 44b5fc853b0..6a50ba7591a 100644 --- a/modules/aaa/mod_auth_digest.c +++ b/modules/aaa/mod_auth_digest.c @@ -261,6 +261,26 @@ static void log_error_and_cleanup(char *msg, apr_status_t sts, server_rec *s) cleanup_tables(NULL); } +/* RMM helper functions that behave like single-step malloc/free. */ + +static void *rmm_malloc(apr_rmm_t *rmm, apr_size_t size) +{ + apr_rmm_off_t offset = apr_rmm_malloc(rmm, size); + + if (!offset) { + return NULL; + } + + return apr_rmm_addr_get(rmm, offset); +} + +static apr_status_t rmm_free(apr_rmm_t *rmm, void *alloc) +{ + apr_rmm_off_t offset = apr_rmm_offset_get(rmm, alloc); + + return apr_rmm_free(rmm, offset); +} + #if APR_HAS_SHARED_MEMORY static int initialize_tables(server_rec *s, apr_pool_t *ctx) @@ -299,8 +319,8 @@ static int initialize_tables(server_rec *s, apr_pool_t *ctx) return !OK; } - client_list = apr_rmm_addr_get(client_rmm, apr_rmm_malloc(client_rmm, sizeof(*client_list) + - sizeof(client_entry*)*num_buckets)); + client_list = rmm_malloc(client_rmm, sizeof(*client_list) + + sizeof(client_entry *) * num_buckets); if (!client_list) { log_error_and_cleanup("failed to allocate shared memory", -1, s); return !OK; @@ -322,7 +342,7 @@ static int initialize_tables(server_rec *s, apr_pool_t *ctx) /* setup opaque */ - opaque_cntr = apr_rmm_addr_get(client_rmm, apr_rmm_malloc(client_rmm, sizeof(*opaque_cntr))); + opaque_cntr = rmm_malloc(client_rmm, sizeof(*opaque_cntr)); if (opaque_cntr == NULL) { log_error_and_cleanup("failed to allocate shared memory", -1, s); return !OK; @@ -339,7 +359,7 @@ static int initialize_tables(server_rec *s, apr_pool_t *ctx) /* setup one-time-nonce counter */ - otn_counter = apr_rmm_addr_get(client_rmm, apr_rmm_malloc(client_rmm, sizeof(*otn_counter))); + otn_counter = rmm_malloc(client_rmm, sizeof(*otn_counter)); if (otn_counter == NULL) { log_error_and_cleanup("failed to allocate shared memory", -1, s); return !OK; @@ -779,7 +799,7 @@ static client_entry *get_client(unsigned long key, const request_rec *r) * last entry in each bucket and updates the counters. Returns the * number of removed entries. */ -static long gc(void) +static long gc(server_rec *s) { client_entry *entry, *prev; unsigned long num_removed = 0, idx; @@ -789,6 +809,12 @@ static long gc(void) for (idx = 0; idx < client_list->tbl_len; idx++) { entry = client_list->table[idx]; prev = NULL; + + if (!entry) { + /* This bucket is empty. */ + continue; + } + while (entry->next) { /* find last entry */ prev = entry; entry = entry->next; @@ -800,8 +826,16 @@ static long gc(void) client_list->table[idx] = NULL; } if (entry) { /* remove entry */ - apr_rmm_free(client_rmm, apr_rmm_offset_get(client_rmm, entry)); + apr_status_t err; + + err = rmm_free(client_rmm, entry); num_removed++; + + if (err) { + /* Nothing we can really do but log... */ + ap_log_error(APLOG_MARK, APLOG_ERR, err, s, APLOGNO() + "Failed to free auth_digest client allocation"); + } } } @@ -835,16 +869,16 @@ static client_entry *add_client(unsigned long key, client_entry *info, /* try to allocate a new entry */ - entry = apr_rmm_addr_get(client_rmm, apr_rmm_malloc(client_rmm, sizeof(client_entry))); + entry = rmm_malloc(client_rmm, sizeof(client_entry)); if (!entry) { - long num_removed = gc(); + long num_removed = gc(s); ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01766) "gc'd %ld client entries. Total new clients: " "%ld; Total removed clients: %ld; Total renewed clients: " "%ld", num_removed, client_list->num_created - client_list->num_renewed, client_list->num_removed, client_list->num_renewed); - entry = apr_rmm_addr_get(client_rmm, apr_rmm_malloc(client_rmm, sizeof(client_entry))); + entry = rmm_malloc(client_rmm, sizeof(client_entry)); if (!entry) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01767) "unable to allocate new auth_digest client");