Bug#655435: libapr1: apr_hash vulnerable to oCert-2011-003 style DOS attacks

To: 655435@bugs.debian.org
Subject: Bug#655435: libapr1: apr_hash vulnerable to oCert-2011-003 style DOS attacks
From: "Jean-Michel Vourgère" <jmv_deb@nirgal.com>
Date: Wed, 22 Feb 2012 00:25:31 +0000
Message-id: <[🔎] 201202220025.31401.jmv_deb@nirgal.com>
Reply-to: "Jean-Michel Vourgère" <jmv_deb@nirgal.com>, 655435@bugs.debian.org

Fixed in 1.4.6

If we need to backport in the future, the changes are located in only two files:
tables/apr_hash.c
test/testhash.c

Attached patch was taken from upstream trunk, it can also apply to previous
versions, including 1.4.2-6+squeeze3

--- tables/apr_hash.c	2012/01/17 06:45:02	1232320
+++ tables/apr_hash.c	2012/01/29 23:38:20	1237507
@@ -18,6 +18,7 @@
 
 #include "apr_general.h"
 #include "apr_pools.h"
+#include "apr_time.h"
 
 #include "apr_hash.h"
 
@@ -75,7 +76,7 @@
     apr_pool_t          *pool;
     apr_hash_entry_t   **array;
     apr_hash_index_t     iterator;  /* For apr_hash_first(NULL, ...) */
-    unsigned int         count, max;
+    unsigned int         count, max, seed;
     apr_hashfunc_t       hash_func;
     apr_hash_entry_t    *free;  /* List of recycled entries */
 };
@@ -95,13 +96,18 @@
 APR_DECLARE(apr_hash_t *) apr_hash_make(apr_pool_t *pool)
 {
     apr_hash_t *ht;
+    apr_time_t now = apr_time_now();
+
     ht = apr_palloc(pool, sizeof(apr_hash_t));
     ht->pool = pool;
     ht->free = NULL;
     ht->count = 0;
     ht->max = INITIAL_MAX;
+    ht->seed = (unsigned int)((now >> 32) ^ now ^ (apr_uintptr_t)pool ^
+                              (apr_uintptr_t)ht ^ (apr_uintptr_t)&now) - 1;
     ht->array = alloc_array(ht, ht->max);
-    ht->hash_func = apr_hashfunc_default;
+    ht->hash_func = NULL;
+
     return ht;
 }
 
@@ -201,10 +207,9 @@
     ht->max = new_max;
 }
 
-APR_DECLARE_NONSTD(unsigned int) apr_hashfunc_default(const char *char_key,
-                                                      apr_ssize_t *klen)
+static unsigned int hashfunc_default(const char *char_key, apr_ssize_t *klen,
+                                     unsigned int hash)
 {
-    unsigned int hash = 0;
     const unsigned char *key = (const unsigned char *)char_key;
     const unsigned char *p;
     apr_ssize_t i;
@@ -246,7 +251,7 @@
      *
      *                  -- Ralf S. Engelschall <rse@engelschall.com>
      */
-     
+
     if (*klen == APR_HASH_KEY_STRING) {
         for (p = key; *p; p++) {
             hash = hash * 33 + *p;
@@ -262,6 +267,11 @@
     return hash;
 }
 
+APR_DECLARE_NONSTD(unsigned int) apr_hashfunc_default(const char *char_key,
+                                                      apr_ssize_t *klen)
+{
+    return hashfunc_default(char_key, klen, 0);
+}
 
 /*
  * This is where we keep the details of the hash function and control
@@ -280,7 +290,10 @@
     apr_hash_entry_t **hep, *he;
     unsigned int hash;
 
-    hash = ht->hash_func(key, &klen);
+    if (ht->hash_func)
+        hash = ht->hash_func(key, &klen);
+    else
+        hash = hashfunc_default(key, &klen, ht->seed);
 
     /* scan linked list */
     for (hep = &ht->array[hash & ht->max], he = *hep;
@@ -322,6 +335,7 @@
     ht->free = NULL;
     ht->count = orig->count;
     ht->max = orig->max;
+    ht->seed = orig->seed;
     ht->hash_func = orig->hash_func;
     ht->array = (apr_hash_entry_t **)((char *)ht + sizeof(apr_hash_t));
 
@@ -419,7 +433,7 @@
     apr_hash_entry_t *new_vals = NULL;
     apr_hash_entry_t *iter;
     apr_hash_entry_t *ent;
-    unsigned int i,j,k;
+    unsigned int i, j, k, hash;
 
 #if APR_POOL_DEBUG
     /* we don't copy keys and values, so it's necessary that
@@ -447,6 +461,7 @@
     if (base->count + overlay->count > res->max) {
         res->max = res->max * 2 + 1;
     }
+    res->seed = base->seed;
     res->array = alloc_array(res, res->max);
     if (base->count + overlay->count) {
         new_vals = apr_palloc(p, sizeof(apr_hash_entry_t) *
@@ -468,7 +483,11 @@
 
     for (k = 0; k <= overlay->max; k++) {
         for (iter = overlay->array[k]; iter; iter = iter->next) {
-            i = iter->hash & res->max;
+            if (res->hash_func)
+                hash = res->hash_func(iter->key, &iter->klen);
+            else
+                hash = hashfunc_default(iter->key, &iter->klen, res->seed);
+            i = hash & res->max;
             for (ent = res->array[i]; ent; ent = ent->next) {
                 if ((ent->klen == iter->klen) &&
                     (memcmp(ent->key, iter->key, iter->klen) == 0)) {
@@ -486,7 +505,7 @@
                 new_vals[j].klen = iter->klen;
                 new_vals[j].key = iter->key;
                 new_vals[j].val = iter->val;
-                new_vals[j].hash = iter->hash;
+                new_vals[j].hash = hash;
                 new_vals[j].next = res->array[i];
                 res->array[i] = &new_vals[j];
                 res->count++;
--- test/testhash.c	2012/01/28 03:00:40	1236967
+++ test/testhash.c	2012/01/28 03:23:11	1236970
@@ -438,6 +438,79 @@
     ABTS_STR_EQUAL(tc, "#entries 5\n", StrArray[5]);
 }
 
+static void overlay_fetch(abts_case *tc, void *data)
+{
+    apr_hash_t *base = NULL;
+    apr_hash_t *overlay = NULL;
+    apr_hash_t *result = NULL;
+    int count;
+
+    base = apr_hash_make(p);
+    overlay = apr_hash_make(p);
+    ABTS_PTR_NOTNULL(tc, base);
+    ABTS_PTR_NOTNULL(tc, overlay);
+
+    apr_hash_set(base, "base1", APR_HASH_KEY_STRING, "value1");
+    apr_hash_set(base, "base2", APR_HASH_KEY_STRING, "value2");
+    apr_hash_set(base, "base3", APR_HASH_KEY_STRING, "value3");
+    apr_hash_set(base, "base4", APR_HASH_KEY_STRING, "value4");
+    apr_hash_set(base, "base5", APR_HASH_KEY_STRING, "value5");
+
+    apr_hash_set(overlay, "overlay1", APR_HASH_KEY_STRING, "value1");
+    apr_hash_set(overlay, "overlay2", APR_HASH_KEY_STRING, "value2");
+    apr_hash_set(overlay, "overlay3", APR_HASH_KEY_STRING, "value3");
+    apr_hash_set(overlay, "overlay4", APR_HASH_KEY_STRING, "value4");
+    apr_hash_set(overlay, "overlay5", APR_HASH_KEY_STRING, "value5");
+
+    result = apr_hash_overlay(p, overlay, base);
+
+    count = apr_hash_count(result);
+    ABTS_INT_EQUAL(tc, 10, count);
+
+    ABTS_STR_EQUAL(tc, "value1",
+                       apr_hash_get(result, "base1", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value2",
+                       apr_hash_get(result, "base2", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value3",
+                       apr_hash_get(result, "base3", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value4",
+                       apr_hash_get(result, "base4", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value5",
+                       apr_hash_get(result, "base5", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value1",
+                       apr_hash_get(result, "overlay1", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value2",
+                       apr_hash_get(result, "overlay2", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value3",
+                       apr_hash_get(result, "overlay3", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value4",
+                       apr_hash_get(result, "overlay4", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value5",
+                       apr_hash_get(result, "overlay5", APR_HASH_KEY_STRING));
+
+    ABTS_STR_EQUAL(tc, "value1",
+                       apr_hash_get(base, "base1", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value2",
+                       apr_hash_get(base, "base2", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value3",
+                       apr_hash_get(base, "base3", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value4",
+                       apr_hash_get(base, "base4", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value5",
+                       apr_hash_get(base, "base5", APR_HASH_KEY_STRING));
+
+    ABTS_STR_EQUAL(tc, "value1",
+                       apr_hash_get(overlay, "overlay1", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value2",
+                       apr_hash_get(overlay, "overlay2", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value3",
+                       apr_hash_get(overlay, "overlay3", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value4",
+                       apr_hash_get(overlay, "overlay4", APR_HASH_KEY_STRING));
+    ABTS_STR_EQUAL(tc, "value5",
+                       apr_hash_get(overlay, "overlay5", APR_HASH_KEY_STRING));
+}
+
 abts_suite *testhash(abts_suite *suite)
 {
     suite = ADD_SUITE(suite)
@@ -461,6 +534,7 @@
     abts_run_test(suite, overlay_empty, NULL);
     abts_run_test(suite, overlay_2unique, NULL);
     abts_run_test(suite, overlay_same, NULL);
+    abts_run_test(suite, overlay_fetch, NULL);
 
     return suite;
 }

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply to:

Prev by Date: Re: How long will it take for 2.4 to enter SID?
Next by Date: Bug#629899: ap_get_local_host is broken (can't always determine the server's FQDN)
Previous by thread: Processed: Re: apache2.2-common: Apache fails to start on boot after upgrade Etch -> Lenny
Next by thread: Bug#629899: ap_get_local_host is broken (can't always determine the server's FQDN)
Index(es):
- Date
- Thread