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 a40867accc4326cec3deee70bf6d431642ffeaa0..db0d12b7da6a3608bece5d4c17222d8c451e1ace 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,11 @@ Changes with Apache 2.4.24
      core: Mitigate [f]cgi "httpoxy" issues.
      [Dominic Scheirlinck <dominic vendhq.com>, 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 <m.malyutin dsec.ru>,
+     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 134f90539aea600405452bea39664d8c77c88478..35076ae2052e13c31daa5393eb1035943924083d 100644 (file)
--- 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 <m.malyutin dsec.ru>,
-     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 44b5fc853b0d504e85c2ca45c93a100e8b89e825..6a50ba7591ab97023f8537eea2e8667d9d883b2b 100644 (file)
--- 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");