Add sanity checks for slotmem size calculations:

* modules/slotmem/mod_slotmem_plain.c,
  modules/slotmem/mod_slotmem_shm.c
  (slotmem_size_mul, slotmem_size_add): New helper functions for
  checked arithmetic on apr_size_t values.
  (slotmem_create): Use checked arithmetic for allocation size
  calculations to prevent integer overflow.
  (slotmem_grab): Add overflow check on size * id multiplication
  before pointer arithmetic.
  (slotmem_get): Validate dest_len against slot size before access,
  move inuse pointer dereference after bounds checks.
  (slotmem_put): Validate src_len against slot size before access,
  move inuse pointer dereference after bounds checks.

* modules/slotmem/mod_slotmem_shm.c (slotmem_fgrab): Add overflow
  check on size * id multiplication before pointer arithmetic.
  (slotmem_attach): Validate shared memory segment size against
  expected size computed with checked arithmetic. Use basesize
  variable for inuse pointer calculation.

Submitted by: metsw24-max <metsw24 gmail.com>
GitHub: closes #626

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1936268 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/slotmem/mod_slotmem_plain.c b/modules/slotmem/mod_slotmem_plain.c
index 4c2b19b61dad7287e2b2b10d19f28b4056985ffa..e3460089641cab2eec2b6e50dd624485217e591e 100644 (file)
--- a/modules/slotmem/mod_slotmem_plain.c
+++ b/modules/slotmem/mod_slotmem_plain.c
@@ -38,6 +38,26 @@ struct ap_slotmem_instance_t {
 static struct ap_slotmem_instance_t *globallistmem = NULL;
 static apr_pool_t *gpool = NULL;
 
+static int slotmem_size_mul(apr_size_t a, apr_size_t b, apr_size_t *res)
+{
+    if (a != 0 && b > ((apr_size_t)-1) / a) {
+        return 0;
+    }
+
+    *res = a * b;
+    return 1;
+}
+
+static int slotmem_size_add(apr_size_t a, apr_size_t b, apr_size_t *res)
+{
+    if (a > ((apr_size_t)-1) - b) {
+        return 0;
+    }
+
+    *res = a + b;
+    return 1;
+}
+
 static apr_status_t slotmem_do(ap_slotmem_instance_t *mem, ap_slotmem_callback_fn_t *func, void *data, apr_pool_t *pool)
 {
     unsigned int i;
@@ -67,10 +87,19 @@ static apr_status_t slotmem_create(ap_slotmem_instance_t **new, const char *name
 {
     ap_slotmem_instance_t *res;
     ap_slotmem_instance_t *next = globallistmem;
-    apr_size_t basesize = (item_size * item_num);
+    apr_size_t basesize;
+    apr_size_t inuse_size;
+    apr_size_t alloc_size;
 
     const char *fname;
 
+    if (!slotmem_size_mul(item_size, (apr_size_t)item_num, &basesize)
+            || !slotmem_size_mul((apr_size_t)item_num, sizeof(char),
+                                 &inuse_size)
+            || !slotmem_size_add(basesize, inuse_size, &alloc_size)) {
+        return APR_EINVAL;
+    }
+
     if (name) {
         if (name[0] == ':')
             fname = name;
@@ -97,7 +126,7 @@ static apr_status_t slotmem_create(ap_slotmem_instance_t **new, const char *name
 
     /* create the memory using the gpool */
     res = (ap_slotmem_instance_t *) apr_pcalloc(gpool, sizeof(ap_slotmem_instance_t));
-    res->base = apr_pcalloc(gpool, basesize + (item_num * sizeof(char)));
+    res->base = apr_pcalloc(gpool, alloc_size);
     if (!res->base)
         return APR_ENOSHMAVAIL;
 
@@ -156,6 +185,10 @@ static apr_status_t slotmem_dptr(ap_slotmem_instance_t *score, unsigned int id,
     if (id >= score->num)
         return APR_EINVAL;
 
+    if (score->size != 0
+            && (apr_size_t)id > ((apr_size_t)-1) / score->size)
+        return APR_EINVAL;
+
     ptr = (char *)score->base + score->size * id;
     if (!ptr)
         return APR_ENOSHMAVAIL;
@@ -172,11 +205,14 @@ static apr_status_t slotmem_get(ap_slotmem_instance_t *slot, unsigned int id, un
     if (!slot) {
         return APR_ENOSHMAVAIL;
     }
-
-    inuse = slot->inuse + id;
     if (id >= slot->num) {
         return APR_EINVAL;
     }
+    if (dest_len > slot->size) {
+        return APR_EINVAL;
+    }
+
+    inuse = slot->inuse + id;
     if (AP_SLOTMEM_IS_PREGRAB(slot) && !*inuse) {
         return APR_NOTFOUND;
     }
@@ -198,11 +234,14 @@ static apr_status_t slotmem_put(ap_slotmem_instance_t *slot, unsigned int id, un
     if (!slot) {
         return APR_ENOSHMAVAIL;
     }
-
-    inuse = slot->inuse + id;
     if (id >= slot->num) {
         return APR_EINVAL;
     }
+    if (src_len > slot->size) {
+        return APR_EINVAL;
+    }
+
+    inuse = slot->inuse + id;
     if (AP_SLOTMEM_IS_PREGRAB(slot) && !*inuse) {
         return APR_NOTFOUND;
     }

diff --git a/modules/slotmem/mod_slotmem_shm.c b/modules/slotmem/mod_slotmem_shm.c
index 4d14faf36b4ac84dca1bba4a094221a7c1dcd651..46aca109706cccdc69e0ee5513f7c8c31c10071a 100644 (file)
--- a/modules/slotmem/mod_slotmem_shm.c
+++ b/modules/slotmem/mod_slotmem_shm.c
@@ -72,6 +72,26 @@ struct ap_slotmem_instance_t {
 static struct ap_slotmem_instance_t *globallistmem = NULL;
 static apr_pool_t *gpool = NULL;
 
+static int slotmem_size_mul(apr_size_t a, apr_size_t b, apr_size_t *res)
+{
+    if (a != 0 && b > ((apr_size_t)-1) / a) {
+        return 0;
+    }
+
+    *res = a * b;
+    return 1;
+}
+
+static int slotmem_size_add(apr_size_t a, apr_size_t b, apr_size_t *res)
+{
+    if (a > ((apr_size_t)-1) - b) {
+        return 0;
+    }
+
+    *res = a + b;
+    return 1;
+}
+
 #define DEFAULT_SLOTMEM_PREFIX "slotmem-shm-"
 #define DEFAULT_SLOTMEM_SUFFIX ".shm"
 #define DEFAULT_SLOTMEM_PERSIST_SUFFIX ".persist"
@@ -348,9 +368,10 @@ static apr_status_t slotmem_create(ap_slotmem_instance_t **new,
     ap_slotmem_instance_t *next = globallistmem;
     const char *fname, *pname = NULL;
     apr_shm_t *shm;
-    apr_size_t basesize = (item_size * item_num);
-    apr_size_t size = AP_SLOTMEM_OFFSET + AP_UNSIGNEDINT_OFFSET +
-                      (item_num * sizeof(char)) + basesize;
+    apr_size_t header_size;
+    apr_size_t basesize;
+    apr_size_t inuse_size;
+    apr_size_t size;
     int persist = (type & AP_SLOTMEM_TYPE_PERSIST) != 0;
     apr_status_t rv;
 
@@ -358,6 +379,14 @@ static apr_status_t slotmem_create(ap_slotmem_instance_t **new,
     if (gpool == NULL) {
         return APR_ENOSHMAVAIL;
     }
+    if (!slotmem_size_mul(item_size, (apr_size_t)item_num, &basesize)
+        || !slotmem_size_mul((apr_size_t)item_num, sizeof(char), &inuse_size)
+        || !slotmem_size_add(AP_SLOTMEM_OFFSET, AP_UNSIGNEDINT_OFFSET,
+                             &header_size)
+        || !slotmem_size_add(header_size, inuse_size, &size)
+        || !slotmem_size_add(size, basesize, &size)) {
+        return APR_EINVAL;
+    }
     if (slotmem_filenames(pool, name, &fname, persist ? &pname : NULL)) {
         /* first try to attach to existing slotmem */
         if (next) {
@@ -483,6 +512,11 @@ static apr_status_t slotmem_attach(ap_slotmem_instance_t **new,
     sharedslotdesc_t *desc;
     const char *fname;
     apr_shm_t *shm;
+    apr_size_t header_size;
+    apr_size_t basesize;
+    apr_size_t inuse_size;
+    apr_size_t expected_size;
+    apr_size_t shm_size;
     apr_status_t rv;
 
     if (gpool == NULL) {
@@ -524,6 +558,23 @@ static apr_status_t slotmem_attach(ap_slotmem_instance_t **new,
 
     /* Read the description of the slotmem */
     desc = (sharedslotdesc_t *)apr_shm_baseaddr_get(shm);
+
+    if (!slotmem_size_mul(desc->size, (apr_size_t)desc->num, &basesize)
+        || !slotmem_size_mul((apr_size_t)desc->num, sizeof(char), &inuse_size)
+        || !slotmem_size_add(AP_SLOTMEM_OFFSET, AP_UNSIGNEDINT_OFFSET,
+                             &header_size)
+        || !slotmem_size_add(header_size, basesize, &expected_size)
+        || !slotmem_size_add(expected_size, inuse_size, &expected_size)) {
+        apr_shm_detach(shm);
+        return APR_EINVAL;
+    }
+
+    shm_size = apr_shm_size_get(shm);
+    if (expected_size > shm_size) {
+        apr_shm_detach(shm);
+        return APR_EINVAL;
+    }
+
     ptr = (char *)desc + AP_SLOTMEM_OFFSET;
 
     /* For the chained slotmem stuff */
@@ -537,7 +588,7 @@ static apr_status_t slotmem_attach(ap_slotmem_instance_t **new,
     res->base = (void *)ptr;
     res->desc = desc;
     res->gpool = gpool;
-    res->inuse = ptr + (desc->size * desc->num);
+    res->inuse = ptr + basesize;
     res->next = NULL;
 
     *new = res;
@@ -562,6 +613,11 @@ static apr_status_t slotmem_dptr(ap_slotmem_instance_t *slot,
         return APR_EINVAL;
     }
 
+    if (slot->desc->size != 0
+            && (apr_size_t)id > ((apr_size_t)-1) / slot->desc->size) {
+        return APR_EINVAL;
+    }
+
     ptr = (char *)slot->base + slot->desc->size * id;
     if (!ptr) {
         return APR_ENOSHMAVAIL;
@@ -580,11 +636,14 @@ static apr_status_t slotmem_get(ap_slotmem_instance_t *slot, unsigned int id,
     if (!slot) {
         return APR_ENOSHMAVAIL;
     }
-
-    inuse = slot->inuse + id;
     if (id >= slot->desc->num) {
         return APR_EINVAL;
     }
+    if (dest_len > slot->desc->size) {
+        return APR_EINVAL;
+    }
+
+    inuse = slot->inuse + id;
     if (AP_SLOTMEM_IS_PREGRAB(slot) && !*inuse) {
         return APR_NOTFOUND;
     }
@@ -607,11 +666,14 @@ static apr_status_t slotmem_put(ap_slotmem_instance_t *slot, unsigned int id,
     if (!slot) {
         return APR_ENOSHMAVAIL;
     }
-
-    inuse = slot->inuse + id;
     if (id >= slot->desc->num) {
         return APR_EINVAL;
     }
+    if (src_len > slot->desc->size) {
+        return APR_EINVAL;
+    }
+
+    inuse = slot->inuse + id;
     if (AP_SLOTMEM_IS_PREGRAB(slot) && !*inuse) {
         return APR_NOTFOUND;
     }