summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLukas Slebodnik <lslebodn@redhat.com>2015-07-30 10:50:47 +0200
committerJakub Hrozek <jhrozek@redhat.com>2015-08-05 11:28:27 +0200
commitba847347cade817ee927397d82c952b51b0dcb2b (patch)
treee7e03dc4cd0a31c9bd632c2a95c7a2e531f07f4e
parentea7839cec593b4a7c678fab52ab864518db6699b (diff)
downloadsssd-ba847347cade817ee927397d82c952b51b0dcb2b.tar.gz
sssd-ba847347cade817ee927397d82c952b51b0dcb2b.tar.xz
sssd-ba847347cade817ee927397d82c952b51b0dcb2b.zip
sss_client: Update integrity check of records in mmap cache
The function sss_nss_mc_get_record return copy of record from memory cache in last argument. Because we should not access data directly to avoid problems with consistency of record. The function sss_nss_mc_get_record also check whether length of record is within data area (with macro MC_CHECK_RECORD_LENGTH) However we also tried to do the same check in functions sss_nss_mc_get{gr, pw}* Pointer to end of strings in record was compared to pointer to the end of data table. But these two pointers are not within the same allocated area and does not make sense to compare them. Sometimes record can be allocated before mmaped area and sometime after. Sometimes it will return cached data and other time will fall back to responder. Resolves: https://fedorahosted.org/sssd/ticket/2743 Reviewed-by: Michal Židek <mzidek@redhat.com>
-rw-r--r--src/sss_client/nss_mc_group.c19
-rw-r--r--src/sss_client/nss_mc_initgr.c26
-rw-r--r--src/sss_client/nss_mc_passwd.c20
3 files changed, 33 insertions, 32 deletions
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index e0fdb97f6..aacf59d9f 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -112,16 +112,16 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
uint32_t hash;
uint32_t slot;
int ret;
- size_t strs_offset;
- uint8_t *max_addr;
+ const size_t strs_offset = offsetof(struct sss_mc_grp_data, strs);
+ size_t data_size;
ret = sss_nss_mc_get_ctx("group", &gr_mc_ctx);
if (ret) {
return ret;
}
- /* Get max address of data table. */
- max_addr = gr_mc_ctx.data_table + gr_mc_ctx.dt_size;
+ /* Get max size of data table. */
+ data_size = gr_mc_ctx.dt_size;
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&gr_mc_ctx, name, name_len + 1);
@@ -130,7 +130,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
/* If slot is not within the bounds of mmaped region and
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
- while (MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
+ while (MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
/* free record from previous iteration */
free(rec);
rec = NULL;
@@ -147,15 +147,16 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
continue;
}
- strs_offset = offsetof(struct sss_mc_grp_data, strs);
data = (struct sss_mc_grp_data *)rec->data;
/* Integrity check
* - name_len cannot be longer than all strings
* - data->name cannot point outside strings
- * - all strings must be within data_table */
+ * - all strings must be within copy of record
+ * - size of record must be lower that data table size */
if (name_len > data->strs_len
|| (data->name + name_len) > (strs_offset + data->strs_len)
- || (uint8_t *)data->strs + data->strs_len > max_addr) {
+ || data->strs_len > rec->len
+ || rec->len > data_size) {
ret = ENOENT;
goto done;
}
@@ -168,7 +169,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
slot = sss_nss_mc_next_slot_with_hash(rec, hash);
}
- if (!MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
ret = ENOENT;
goto done;
}
diff --git a/src/sss_client/nss_mc_initgr.c b/src/sss_client/nss_mc_initgr.c
index 153617ea9..74143d9fb 100644
--- a/src/sss_client/nss_mc_initgr.c
+++ b/src/sss_client/nss_mc_initgr.c
@@ -94,15 +94,15 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
uint32_t slot;
int ret;
const size_t data_offset = offsetof(struct sss_mc_initgr_data, gids);
- uint8_t *max_addr;
+ size_t data_size;
ret = sss_nss_mc_get_ctx("initgroups", &initgr_mc_ctx);
if (ret) {
return ret;
}
- /* Get max address of data table. */
- max_addr = initgr_mc_ctx.data_table + initgr_mc_ctx.dt_size;
+ /* Get max size of data table. */
+ data_size = initgr_mc_ctx.dt_size;
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&initgr_mc_ctx, name, name_len + 1);
@@ -111,7 +111,7 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
/* If slot is not within the bounds of mmaped region and
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
- while (MC_SLOT_WITHIN_BOUNDS(slot, initgr_mc_ctx.dt_size)) {
+ while (MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
/* free record from previous iteration */
free(rec);
rec = NULL;
@@ -132,14 +132,14 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
rec_name = (char *)data + data->name;
/* Integrity check
* - name_len cannot be longer than all strings or data
- * - data->name cannot point outside strings
- * - all data must be within data_table
- * - name must be within data_table */
- if (name_len > data->data_len
- || name_len > data->strs_len
- || (data->strs + name_len) > (data_offset + data->data_len)
- || (uint8_t *)data->gids + data->data_len > max_addr
- || (uint8_t *)rec_name + name_len > max_addr) {
+ * - all data must be within copy of record
+ * - size of record must be lower that data table size
+ * - data->strs cannot point outside strings */
+ if (name_len > data->strs_len
+ || data->strs_len > data->data_len
+ || data->data_len > rec->len
+ || rec->len > data_size
+ || (data->strs + name_len) > (data_offset + data->data_len)) {
ret = ENOENT;
goto done;
}
@@ -151,7 +151,7 @@ errno_t sss_nss_mc_initgroups_dyn(const char *name, size_t name_len,
slot = sss_nss_mc_next_slot_with_hash(rec, hash);
}
- if (!MC_SLOT_WITHIN_BOUNDS(slot, initgr_mc_ctx.dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
ret = ENOENT;
goto done;
}
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index 10e43e2af..0da7ad0ae 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -105,16 +105,16 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
uint32_t hash;
uint32_t slot;
int ret;
- size_t strs_offset;
- uint8_t *max_addr;
+ const size_t strs_offset = offsetof(struct sss_mc_pwd_data, strs);
+ size_t data_size;
ret = sss_nss_mc_get_ctx("passwd", &pw_mc_ctx);
if (ret) {
return ret;
}
- /* Get max address of data table. */
- max_addr = pw_mc_ctx.data_table + pw_mc_ctx.dt_size;
+ /* Get max size of data table. */
+ data_size = pw_mc_ctx.dt_size;
/* hashes are calculated including the NULL terminator */
hash = sss_nss_mc_hash(&pw_mc_ctx, name, name_len + 1);
@@ -123,7 +123,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
/* If slot is not within the bounds of mmaped region and
* it's value is not MC_INVALID_VAL, then the cache is
* probbably corrupted. */
- while (MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
+ while (MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
/* free record from previous iteration */
free(rec);
rec = NULL;
@@ -140,16 +140,16 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
continue;
}
- strs_offset = offsetof(struct sss_mc_pwd_data, strs);
-
data = (struct sss_mc_pwd_data *)rec->data;
/* Integrity check
* - name_len cannot be longer than all strings
* - data->name cannot point outside strings
- * - all strings must be within data_table */
+ * - all strings must be within copy of record
+ * - size of record must be lower that data table size */
if (name_len > data->strs_len
|| (data->name + name_len) > (strs_offset + data->strs_len)
- || (uint8_t *)data->strs + data->strs_len > max_addr) {
+ || data->strs_len > rec->len
+ || rec->len > data_size) {
ret = ENOENT;
goto done;
}
@@ -162,7 +162,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
slot = sss_nss_mc_next_slot_with_hash(rec, hash);
}
- if (!MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
+ if (!MC_SLOT_WITHIN_BOUNDS(slot, data_size)) {
ret = ENOENT;
goto done;
}