Skip to content

harden size calculations, bounds checks, and SHM layout validation #626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
metsw24-max wants to merge 1 commit into
base: trunk
Choose a base branch
from
+117 −14
Open

harden size calculations, bounds checks, and SHM layout validation #626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 45 additions & 6 deletions modules/slotmem/mod_slotmem_plain.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
80 changes: 72 additions & 8 deletions modules/slotmem/mod_slotmem_shm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -348,16 +368,26 @@ 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;

*new = NULL;
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) {
Expand Down Expand Up @@ -483,6 +513,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) {
Expand Down Expand Up @@ -524,6 +559,24 @@ 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 */
Expand All @@ -537,7 +590,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;
Expand All @@ -562,6 +615,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;
Expand All @@ -580,11 +638,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;
}
Expand All @@ -607,11 +668,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;
}
Expand Down