[SSSD] [PATCH] Copy pam data from DBus message

Sumit Bose sbose at redhat.com
Thu May 20 08:24:49 UTC 2010


Hi,

although I'm still not sure where the related DBus message is freed this
patch fixes the potential issue of accessing already freed data by
copying it out of the message.

bye,
Sumit
-------------- next part --------------
From fa2663d1c267870fca905404c6e718447ead538b Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 20 May 2010 10:12:47 +0200
Subject: [PATCH] Copy pam data from DBus message

Instead of just using references to the pam data inside of the DBus
message the data is copied. New the DBus message can be freed at any
time and the pam data is part of the memory hierarchy. Additionally it
is possible to overwrite the authentication tokens in the DBus message,
because it is not used elsewhere.
---
 src/providers/data_provider.h    |    4 +-
 src/providers/data_provider_be.c |   17 ++----
 src/providers/dp_auth_util.c     |  106 +++++++++++++++++++++++---------------
 3 files changed, 73 insertions(+), 54 deletions(-)

diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h
index 951b47a..e71d130 100644
--- a/src/providers/data_provider.h
+++ b/src/providers/data_provider.h
@@ -198,8 +198,8 @@ int pam_add_response(struct pam_data *pd,
                      int len, const uint8_t *data);
 
 bool dp_pack_pam_request(DBusMessage *msg, struct pam_data *pd);
-bool dp_unpack_pam_request(DBusMessage *msg, struct pam_data *pd,
-                           DBusError *dbus_error);
+bool dp_unpack_pam_request(DBusMessage *msg, TALLOC_CTX *mem_ctx,
+                           struct pam_data **new_pd, DBusError *dbus_error);
 
 bool dp_pack_pam_response(DBusMessage *msg, struct pam_data *pd);
 bool dp_unpack_pam_response(DBusMessage *msg, struct pam_data *pd,
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 4ece1b6..10328a0 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -536,10 +536,13 @@ static int be_pam_handler(DBusMessage *message, struct sbus_connection *conn)
     be_req->fn = be_pam_handler_callback;
     be_req->pvt = reply;
 
-    pd = talloc_zero(be_req, struct pam_data);
-    if (!pd) {
+    dbus_error_init(&dbus_error);
+
+    ret = dp_unpack_pam_request(message, be_req, &pd, &dbus_error);
+    if (!ret) {
+        DEBUG(1,("Failed, to parse message!\n"));
         talloc_free(be_req);
-        return ENOMEM;
+        return EIO;
     }
 
     pd->pam_status = PAM_SYSTEM_ERR;
@@ -549,14 +552,6 @@ static int be_pam_handler(DBusMessage *message, struct sbus_connection *conn)
         return ENOMEM;
     }
 
-    dbus_error_init(&dbus_error);
-
-    ret = dp_unpack_pam_request(message, pd, &dbus_error);
-    if (!ret) {
-        DEBUG(1,("Failed, to parse message!\n"));
-        talloc_free(be_req);
-        return EIO;
-    }
 
     DEBUG(4, ("Got request with the following data\n"));
     DEBUG_PAM_DATA(4, pd);
diff --git a/src/providers/dp_auth_util.c b/src/providers/dp_auth_util.c
index e09a692..2a81ebf 100644
--- a/src/providers/dp_auth_util.c
+++ b/src/providers/dp_auth_util.c
@@ -23,7 +23,7 @@
 
 bool dp_pack_pam_request(DBusMessage *msg, struct pam_data *pd)
 {
-    int ret;
+    dbus_bool_t db_ret;
 
     if (pd->user == NULL) return false;
     if (pd->service == NULL) pd->service = talloc_strdup(pd, "");
@@ -32,52 +32,76 @@ bool dp_pack_pam_request(DBusMessage *msg, struct pam_data *pd)
     if (pd->rhost == NULL) pd->rhost = talloc_strdup(pd, "");
 
 
-    ret = dbus_message_append_args(msg,
-                                   DBUS_TYPE_INT32,  &(pd->cmd),
-                                   DBUS_TYPE_STRING, &(pd->user),
-                                   DBUS_TYPE_STRING, &(pd->service),
-                                   DBUS_TYPE_STRING, &(pd->tty),
-                                   DBUS_TYPE_STRING, &(pd->ruser),
-                                   DBUS_TYPE_STRING, &(pd->rhost),
-                                   DBUS_TYPE_UINT32, &(pd->authtok_type),
+    db_ret = dbus_message_append_args(msg,
+                                      DBUS_TYPE_INT32,  &(pd->cmd),
+                                      DBUS_TYPE_STRING, &(pd->user),
+                                      DBUS_TYPE_STRING, &(pd->service),
+                                      DBUS_TYPE_STRING, &(pd->tty),
+                                      DBUS_TYPE_STRING, &(pd->ruser),
+                                      DBUS_TYPE_STRING, &(pd->rhost),
+                                      DBUS_TYPE_UINT32, &(pd->authtok_type),
+                                      DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE,
+                                          &(pd->authtok),
+                                          (pd->authtok_size),
+                                      DBUS_TYPE_UINT32, &(pd->newauthtok_type),
+                                      DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE,
+                                          &(pd->newauthtok),
+                                          pd->newauthtok_size,
+                                      DBUS_TYPE_INT32, &(pd->priv),
+                                      DBUS_TYPE_UINT32, &(pd->cli_pid),
+                                      DBUS_TYPE_INVALID);
+
+    return db_ret;
+}
+
+bool dp_unpack_pam_request(DBusMessage *msg, TALLOC_CTX *mem_ctx,
+                           struct pam_data **new_pd, DBusError *dbus_error)
+{
+    dbus_bool_t db_ret;
+    int ret;
+    struct pam_data pd;
+
+    db_ret = dbus_message_get_args(msg, dbus_error,
+                                   DBUS_TYPE_INT32,  &(pd.cmd),
+                                   DBUS_TYPE_STRING, &(pd.user),
+                                   DBUS_TYPE_STRING, &(pd.service),
+                                   DBUS_TYPE_STRING, &(pd.tty),
+                                   DBUS_TYPE_STRING, &(pd.ruser),
+                                   DBUS_TYPE_STRING, &(pd.rhost),
+                                   DBUS_TYPE_UINT32, &(pd.authtok_type),
                                    DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE,
-                                       &(pd->authtok),
-                                       (pd->authtok_size),
-                                   DBUS_TYPE_UINT32, &(pd->newauthtok_type),
+                                       &(pd.authtok),
+                                       &(pd.authtok_size),
+                                   DBUS_TYPE_UINT32, &(pd.newauthtok_type),
                                    DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE,
-                                       &(pd->newauthtok),
-                                       pd->newauthtok_size,
-                                   DBUS_TYPE_INT32, &(pd->priv),
-                                   DBUS_TYPE_UINT32, &(pd->cli_pid),
+                                       &(pd.newauthtok),
+                                       &(pd.newauthtok_size),
+                                   DBUS_TYPE_INT32, &(pd.priv),
+                                   DBUS_TYPE_UINT32, &(pd.cli_pid),
                                    DBUS_TYPE_INVALID);
 
-    return ret;
-}
+    if (!db_ret) {
+        DEBUG(1, ("dbus_message_get_args failed.\n"));
+        return false;
+    }
 
-bool dp_unpack_pam_request(DBusMessage *msg, struct pam_data *pd, DBusError *dbus_error)
-{
-    int ret;
+    ret = copy_pam_data(mem_ctx, &pd, new_pd);
+    if (ret != EOK) {
+        DEBUG(1, ("copy_pam_data failed.\n"));
+        return false;
+    }
 
-    ret = dbus_message_get_args(msg, dbus_error,
-                                DBUS_TYPE_INT32,  &(pd->cmd),
-                                DBUS_TYPE_STRING, &(pd->user),
-                                DBUS_TYPE_STRING, &(pd->service),
-                                DBUS_TYPE_STRING, &(pd->tty),
-                                DBUS_TYPE_STRING, &(pd->ruser),
-                                DBUS_TYPE_STRING, &(pd->rhost),
-                                DBUS_TYPE_UINT32, &(pd->authtok_type),
-                                DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE,
-                                    &(pd->authtok),
-                                    &(pd->authtok_size),
-                                DBUS_TYPE_UINT32, &(pd->newauthtok_type),
-                                DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE,
-                                    &(pd->newauthtok),
-                                    &(pd->newauthtok_size),
-                                DBUS_TYPE_INT32, &(pd->priv),
-                                DBUS_TYPE_UINT32, &(pd->cli_pid),
-                                DBUS_TYPE_INVALID);
-
-    return ret;
+    if (pd.authtok_size != 0 && pd.authtok != NULL) {
+        memset(pd.authtok, 0, pd.authtok_size);
+        pd.authtok_size = 0;
+    }
+
+    if (pd.newauthtok_size != 0 && pd.newauthtok != NULL) {
+        memset(pd.newauthtok, 0, pd.newauthtok_size);
+        pd.newauthtok_size = 0;
+    }
+
+    return true;
 }
 
 bool dp_pack_pam_response(DBusMessage *msg, struct pam_data *pd)
-- 
1.6.6.1



More information about the sssd-devel mailing list