From e61f36189c4354a202f2b3bee86a026c7080690f Mon Sep 17 00:00:00 2001

From: Alexey Tikhonov <atikhono@redhat.com>

Date: Tue, 29 Jan 2019 12:13:30 +0100

Subject: [PATCH 5/9] providers/proxy: fixed erroneous free of orig_grp

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Function `remove_duplicate_group_members(mem_ctx, orig_grp, new_grp)`

in case of empty orig_grp would return as a result:

```

*new_grp = talloc_steal(mem_ctx, orig_grp);

```

Since mem_ctx is freed in caller function that leads to deallocation

of orig_grp and to "use after free" bug.

Code was changes so remove_duplicate_group_members() behaves consistently

and always returns a new group created in given mem context.

Reviewed-by: Pavel Březina <pbrezina@redhat.com>

cherry picked from commit cd1538bc92a319a19915d3a19efd9d5e013f7e75

and amended with

commit e1b678c0cce73494d986610920b03956c1dbb62a by Lukas Slebodnik

Reviewed-by: Pavel Březina <pbrezina@redhat.com>

---

 src/providers/proxy/proxy_id.c | 70 ++++++++++++++++++----------------

 1 file changed, 38 insertions(+), 32 deletions(-)

diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c

index 3e8a43ad7..25e866bfb 100644

--- a/src/providers/proxy/proxy_id.c

+++ b/src/providers/proxy/proxy_id.c

@@ -597,8 +597,32 @@ static errno_t remove_duplicate_group_members(TALLOC_CTX *mem_ctx,

         return ENOMEM;

     }

+    grp = talloc(tmp_ctx, struct group);

+    if (grp == NULL) {

+        DEBUG(SSSDBG_OP_FAILURE, "talloc failed.\n");

+        ret = ENOMEM;

+        goto done;

+    }

+

+    grp->gr_gid = orig_grp->gr_gid;

+

+    grp->gr_name = talloc_strdup(grp, orig_grp->gr_name);

+    if (grp->gr_name == NULL) {

+        DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");

+        ret = ENOMEM;

+        goto done;

+    }

+

+    grp->gr_passwd = talloc_strdup(grp, orig_grp->gr_passwd);

+    if (grp->gr_passwd == NULL) {

+        DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");

+        ret = ENOMEM;

+        goto done;

+    }

+

     if (orig_grp->gr_mem == NULL) {

-        ret = ENOENT;

+        grp->gr_mem = NULL;

+        ret = EOK;

         goto done;

     }

@@ -607,7 +631,14 @@ static errno_t remove_duplicate_group_members(TALLOC_CTX *mem_ctx,

     orig_member_count = i;

     if (orig_member_count == 0) {

-        ret = ENOENT;

+        grp->gr_mem = talloc_zero_array(grp, char *, 1);

+        if (grp->gr_mem == NULL) {

+            DEBUG(SSSDBG_OP_FAILURE, "talloc_zero_array failed.\n");

+            ret = ENOMEM;

+            goto done;

+        }

+        grp->gr_mem[0] = NULL;

+        ret = EOK;

         goto done;

     }

@@ -636,14 +667,8 @@ static errno_t remove_duplicate_group_members(TALLOC_CTX *mem_ctx,

     member_count = hash_count(member_tbl);

     if (member_count == 0) {

-        ret = ENOENT;

-        goto done;

-    }

-

-    grp = talloc(tmp_ctx, struct group);

-    if (grp == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "talloc failed.\n");

-        ret = ENOMEM;

+        DEBUG(SSSDBG_CRIT_FAILURE, "Empty resulting hash table - must be internal bug.\n");

+        ret = EINVAL;

         goto done;

     }

@@ -673,32 +698,13 @@ static errno_t remove_duplicate_group_members(TALLOC_CTX *mem_ctx,

     }

     grp->gr_mem[i] = NULL;

-    grp->gr_gid = orig_grp->gr_gid;

-

-    grp->gr_name = talloc_strdup(grp, orig_grp->gr_name);

-    if (grp->gr_name == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");

-        ret = ENOMEM;

-        goto done;

-    }

-

-    grp->gr_passwd = talloc_strdup(grp, orig_grp->gr_passwd);

-    if (grp->gr_passwd == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");

-        ret = ENOMEM;

-        goto done;

-    }

-

-    *_grp = talloc_steal(mem_ctx, grp);

     ret = EOK;

 done:

-    talloc_zfree(tmp_ctx);

-

-    if (ret == ENOENT) {

-        *_grp = talloc_steal(mem_ctx, orig_grp);

-        ret = EOK;

+    if (ret == EOK) {

+        *_grp = talloc_steal(mem_ctx, grp);

     }

+    talloc_zfree(tmp_ctx);

     return ret;

 }

-- 

2.21.1