On 08/15/2013 06:38 PM, Michal Židek wrote:
On 08/15/2013 05:31 PM, Simo Sorce wrote:
On Thu, 2013-08-15 at 17:08 +0200, Michal Židek wrote:
+static size_t sss_mc_get_strs_offset(struct sss_mc_ctx *mcc)
+{
+    if (mcc->type == SSS_MC_PASSWD) {
+        return offsetof(struct sss_mc_pwd_data, strs);
+    }
+
+    return offsetof(struct sss_mc_grp_data, strs);
+}

Can you use a switch()/case: here?

I know it looks a lot more boilerplate but if we start adding other maps
we'll not risk returning a completely bogus pointer by mistake.

Simo.


Ok. New patches are attached.

Thanks
Michal


Actually, I think it looks better without the ret variable. So again,
new patches are attached.

Michal

>From 8544f2eab5d20d082cdb3788ef07ebaa3b3157c5 Mon Sep 17 00:00:00 2001
From: Michal Zidek <[email protected]>
Date: Mon, 12 Aug 2013 19:29:56 +0200
Subject: [PATCH 1/4] mmap_cache: Check data->name value in client code

data->name value must be checked to prevent segfaults in
case of corrupted memory cache.

resolves:
https://fedorahosted.org/sssd/ticket/2018
---
 src/sss_client/nss_mc_group.c  | 18 ++++++++++++++++++
 src/sss_client/nss_mc_passwd.c | 19 +++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index 2d69be9..da5da04 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -23,6 +23,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <sys/mman.h>
 #include <time.h>
 #include "nss_mc.h"
@@ -102,12 +103,17 @@ 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;
 
     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;
+
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&gr_mc_ctx, name, name_len + 1);
     slot = gr_mc_ctx.hash_table[hash];
@@ -133,7 +139,19 @@ 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 */
+        if (name_len > data->strs_len
+            || (data->name + name_len) > (strs_offset + data->strs_len)
+            || (uint8_t *)data->strs + data->strs_len > max_addr) {
+            ret = ENOENT;
+            goto done;
+        }
+
         rec_name = (char *)data + data->name;
         if (strcmp(name, rec_name) == 0) {
             break;
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index fa21bd2..4b08766 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -23,6 +23,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <sys/mman.h>
 #include <time.h>
 #include "nss_mc.h"
@@ -103,12 +104,17 @@ 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;
 
     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;
+
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&pw_mc_ctx, name, name_len + 1);
     slot = pw_mc_ctx.hash_table[hash];
@@ -134,7 +140,20 @@ 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 */
+        if (name_len > data->strs_len
+            || (data->name + name_len) > (strs_offset + data->strs_len)
+            || (uint8_t *)data->strs + data->strs_len > max_addr) {
+            ret = ENOENT;
+            goto done;
+        }
+
         rec_name = (char *)data + data->name;
         if (strcmp(name, rec_name) == 0) {
             break;
-- 
1.7.11.2

>From 9688783586aae89bf7da5923fd7a7de8b6f6694d Mon Sep 17 00:00:00 2001
From: Michal Zidek <[email protected]>
Date: Wed, 14 Aug 2013 18:01:30 +0200
Subject: [PATCH 2/4] mmap_cache: Remove triple checks in client code.

We had pattern in client code with 3 conditions
that can be replaced with one.
---
 src/sss_client/nss_mc_group.c  | 30 ++++++++++--------------------
 src/sss_client/nss_mc_passwd.c | 30 ++++++++++--------------------
 2 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index da5da04..9fe72a6 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -117,16 +117,11 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&gr_mc_ctx, name, name_len + 1);
     slot = gr_mc_ctx.hash_table[hash];
-    if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
-        return ENOENT;
-    }
-
-    while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
-            /* This probably means that the memory cache was corrupted. */
-            return ENOENT;
-        }
 
+    /* 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 (slot < MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
         ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -160,7 +155,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
         slot = rec->next;
     }
 
-    if (slot == MC_INVALID_VAL) {
+    if (slot >= MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
         ret = ENOENT;
         goto done;
     }
@@ -197,16 +192,11 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&gr_mc_ctx, gidstr, len+1);
     slot = gr_mc_ctx.hash_table[hash];
-    if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
-        return ENOENT;
-    }
-
-    while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
-            /* This probably means that the memory cache was corrupted. */
-            return ENOENT;
-        }
 
+    /* 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 (slot < MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
         ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -227,7 +217,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
         slot = rec->next;
     }
 
-    if (slot == MC_INVALID_VAL) {
+    if (slot >= MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
         ret = ENOENT;
         goto done;
     }
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index 4b08766..7aca4a0 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -118,16 +118,11 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&pw_mc_ctx, name, name_len + 1);
     slot = pw_mc_ctx.hash_table[hash];
-    if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
-        return ENOENT;
-    }
-
-    while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
-            /* This probably means that the memory cache was corrupted */
-            return ENOENT;
-        }
 
+    /* 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 (slot < MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
         ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -162,7 +157,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
         slot = rec->next;
     }
 
-    if (slot == MC_INVALID_VAL) {
+    if (slot >= MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
         ret = ENOENT;
         goto done;
     }
@@ -199,16 +194,11 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
     /* hashes are calculated including the NULL terminator */
     hash = sss_nss_mc_hash(&pw_mc_ctx, uidstr, len+1);
     slot = pw_mc_ctx.hash_table[hash];
-    if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
-        return ENOENT;
-    }
-
-    while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
-            /* This probably means that the memory cache was corrupted */
-            return ENOENT;
-        }
 
+    /* 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 (slot < MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
         ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -229,7 +219,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
         slot = rec->next;
     }
 
-    if (slot == MC_INVALID_VAL) {
+    if (slot >= MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
         ret = ENOENT;
         goto done;
     }
-- 
1.7.11.2

>From 5a3dd0209949966a2199fad3e7e380180e4f9cf6 Mon Sep 17 00:00:00 2001
From: Michal Zidek <[email protected]>
Date: Wed, 14 Aug 2013 18:22:06 +0200
Subject: [PATCH 3/4] mmap_cache: Of by one error.

Removes off by one error when using macro MC_SIZE_TO_SLOTS
and adds new macro MC_SLOT_WITHIN_BOUNDS.
---
 src/responder/nss/nsssrv_mmap_cache.c | 12 ++++++------
 src/sss_client/nss_mc_group.c         |  8 ++++----
 src/sss_client/nss_mc_passwd.c        |  8 ++++----
 src/util/mmap_cache.h                 |  3 +++
 4 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index cd5a643..a1bab0c 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -368,12 +368,12 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
     hash = sss_mc_hash(mcc, key->str, key->len);
 
     slot = mcc->hash_table[hash];
-    if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
         return NULL;
     }
 
     while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+        if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
             DEBUG(SSSDBG_FATAL_FAILURE,
                   ("Corrupted fastcache. Slot number too big.\n"));
             sss_mmap_cache_reset(mcc);
@@ -617,13 +617,13 @@ errno_t sss_mmap_cache_pw_invalidate_uid(struct sss_mc_ctx *mcc, uid_t uid)
     hash = sss_mc_hash(mcc, uidstr, strlen(uidstr) + 1);
 
     slot = mcc->hash_table[hash];
-    if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
         ret = ENOENT;
         goto done;
     }
 
     while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+        if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
             DEBUG(SSSDBG_FATAL_FAILURE, ("Corrupted fastcache.\n"));
             sss_mmap_cache_reset(mcc);
             ret = ENOENT;
@@ -755,13 +755,13 @@ errno_t sss_mmap_cache_gr_invalidate_gid(struct sss_mc_ctx *mcc, gid_t gid)
     hash = sss_mc_hash(mcc, gidstr, strlen(gidstr) + 1);
 
     slot = mcc->hash_table[hash];
-    if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
         ret = ENOENT;
         goto done;
     }
 
     while (slot != MC_INVALID_VAL) {
-        if (slot > MC_SIZE_TO_SLOTS(mcc->dt_size)) {
+        if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
             DEBUG(SSSDBG_FATAL_FAILURE, ("Corrupted fastcache.\n"));
             sss_mmap_cache_reset(mcc);
             ret = ENOENT;
diff --git a/src/sss_client/nss_mc_group.c b/src/sss_client/nss_mc_group.c
index 9fe72a6..4e3d9fb 100644
--- a/src/sss_client/nss_mc_group.c
+++ b/src/sss_client/nss_mc_group.c
@@ -121,7 +121,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 (slot < MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+    while (MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
         ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -155,7 +155,7 @@ errno_t sss_nss_mc_getgrnam(const char *name, size_t name_len,
         slot = rec->next;
     }
 
-    if (slot >= MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
         ret = ENOENT;
         goto done;
     }
@@ -196,7 +196,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
     /* 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 (slot < MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+    while (MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
         ret = sss_nss_mc_get_record(&gr_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -217,7 +217,7 @@ errno_t sss_nss_mc_getgrgid(gid_t gid,
         slot = rec->next;
     }
 
-    if (slot >= MC_SIZE_TO_SLOTS(gr_mc_ctx.dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, gr_mc_ctx.dt_size)) {
         ret = ENOENT;
         goto done;
     }
diff --git a/src/sss_client/nss_mc_passwd.c b/src/sss_client/nss_mc_passwd.c
index 7aca4a0..a0a8d87 100644
--- a/src/sss_client/nss_mc_passwd.c
+++ b/src/sss_client/nss_mc_passwd.c
@@ -122,7 +122,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 (slot < MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+    while (MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
         ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -157,7 +157,7 @@ errno_t sss_nss_mc_getpwnam(const char *name, size_t name_len,
         slot = rec->next;
     }
 
-    if (slot >= MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
         ret = ENOENT;
         goto done;
     }
@@ -198,7 +198,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
     /* 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 (slot < MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+    while (MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
         ret = sss_nss_mc_get_record(&pw_mc_ctx, slot, &rec);
         if (ret) {
             goto done;
@@ -219,7 +219,7 @@ errno_t sss_nss_mc_getpwuid(uid_t uid,
         slot = rec->next;
     }
 
-    if (slot >= MC_SIZE_TO_SLOTS(pw_mc_ctx.dt_size)) {
+    if (!MC_SLOT_WITHIN_BOUNDS(slot, pw_mc_ctx.dt_size)) {
         ret = ENOENT;
         goto done;
     }
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index 6c223df..abf8cac 100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -67,6 +67,9 @@ typedef uint32_t rel_ptr_t;
 #define MC_SLOT_TO_PTR(base, slot, type) \
                                 (type *)((base) + ((slot) * MC_SLOT_SIZE))
 
+#define MC_SLOT_WITHIN_BOUNDS(slot, dt_size) \
+    ((slot) < ((dt_size) / MC_SLOT_SIZE))
+
 #define MC_VALID_BARRIER(val) (((val) & 0xff000000) == 0xf0000000)
 
 #define MC_CHECK_RECORD_LENGTH(mc_ctx, rec) \
-- 
1.7.11.2

>From fcbfe80a6011c3d0e365348f2944b8df0582ff89 Mon Sep 17 00:00:00 2001
From: Michal Zidek <[email protected]>
Date: Thu, 15 Aug 2013 16:08:17 +0200
Subject: [PATCH 4/4] mmap_cache: Use better checks for corrupted mc in
 responder

We introduced new way to check integrity of memcache in the
client code. We should use similiar checks in the responder.
---
 src/responder/nss/nsssrv_mmap_cache.c | 56 +++++++++++++++++++++++++++++++++--
 src/util/mmap_cache.h                 |  2 --
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index a1bab0c..95a7fe9 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -356,6 +356,39 @@ static errno_t sss_mc_find_free_slots(struct sss_mc_ctx *mcc,
     return EOK;
 }
 
+static errno_t sss_mc_get_strs_offset(struct sss_mc_ctx *mcc,
+                                      size_t *_offset)
+{
+    switch (mcc->type) {
+    case SSS_MC_PASSWD:
+        *_offset = offsetof(struct sss_mc_pwd_data, strs);
+        return EOK;
+    case SSS_MC_GROUP:
+        *_offset = offsetof(struct sss_mc_grp_data, strs);
+        return EOK;
+    default:
+        DEBUG(SSSDBG_FATAL_FAILURE, ("Unknown memory cache type.\n"));
+        return EINVAL;
+    }
+}
+
+static errno_t sss_mc_get_strs_len(struct sss_mc_ctx *mcc,
+                                   struct sss_mc_rec *rec,
+                                   size_t *_len)
+{
+    switch (mcc->type) {
+    case SSS_MC_PASSWD:
+        *_len = ((struct sss_mc_pwd_data *)&rec->data)->strs_len;
+        return EOK;
+    case SSS_MC_GROUP:
+        *_len = ((struct sss_mc_grp_data *)&rec->data)->strs_len;
+        return EOK;
+    default:
+        DEBUG(SSSDBG_FATAL_FAILURE, ("Unknown memory cache type.\n"));
+        return EINVAL;
+    }
+}
+
 static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
                                              struct sized_string *key)
 {
@@ -364,6 +397,10 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
     uint32_t slot;
     rel_ptr_t name_ptr;
     char *t_key;
+    size_t strs_offset;
+    size_t strs_len;
+    uint8_t *max_addr;
+    errno_t ret;
 
     hash = sss_mc_hash(mcc, key->str, key->len);
 
@@ -372,6 +409,14 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
         return NULL;
     }
 
+    /* Get max address of data table. */
+    max_addr = mcc->data_table + mcc->dt_size;
+
+    ret = sss_mc_get_strs_offset(mcc, &strs_offset);
+    if (ret != EOK) {
+        return NULL;
+    }
+
     while (slot != MC_INVALID_VAL) {
         if (!MC_SLOT_WITHIN_BOUNDS(slot, mcc->dt_size)) {
             DEBUG(SSSDBG_FATAL_FAILURE,
@@ -381,10 +426,15 @@ static struct sss_mc_rec *sss_mc_find_record(struct sss_mc_ctx *mcc,
         }
 
         rec = MC_SLOT_TO_PTR(mcc->data_table, slot, struct sss_mc_rec);
+        ret = sss_mc_get_strs_len(mcc, rec, &strs_len);
+        if (ret != EOK) {
+            return NULL;
+        }
+
         name_ptr = *((rel_ptr_t *)rec->data);
-        /* FIXME: This check relies on fact that offset of member strs
-         * is the same in structures sss_mc_pwd_data and sss_mc_group_data. */
-        if (name_ptr != offsetof(struct sss_mc_pwd_data, strs)) {
+        if (key->len > strs_len
+            || (name_ptr + key->len) > (strs_offset + strs_len)
+            || (uint8_t *)rec->data + strs_offset + strs_len > max_addr) {
             DEBUG(SSSDBG_FATAL_FAILURE,
                   ("Corrupted fastcache. name_ptr value is %u.\n", name_ptr));
             sss_mmap_cache_reset(mcc);
diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h
index abf8cac..7c6693a 100644
--- a/src/util/mmap_cache.h
+++ b/src/util/mmap_cache.h
@@ -113,8 +113,6 @@ struct sss_mc_rec {
     char data[0];
 };
 
-/* FIXME: Function sss_mc_find_record currently relies on fact that
- * offset of strs is the same in both sss_mc_pwd_data and sss_mc_grp_data. */
 struct sss_mc_pwd_data {
     rel_ptr_t name;         /* ptr to name string, rel. to struct base addr */
     uint32_t uid;
-- 
1.7.11.2

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to