Commit db5fcaac authored by Jim Jagielski's avatar Jim Jagielski
Browse files

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
parent 4ba157f5
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -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
+0 −7
Original line number Diff line number Diff line
@@ -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 ]
+43 −9
Original line number Diff line number Diff line
@@ -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");