tasn pushed a commit to branch master.

http://git.enlightenment.org/core/enlightenment.git/commit/?id=acfdda6c7fbfdb376613b47b5980642877e1e25c

commit acfdda6c7fbfdb376613b47b5980642877e1e25c
Author: Tom Hacohen <t...@stosb.com>
Date:   Tue Apr 21 10:07:42 2015 +0100

    E auth: improve clearing out passwords from memory.
    
    Optimising compilers (like gcc/clang with -O1 or above) were optimising
    out the memset(). Until link time optimisations are good enough, this
    will prevent them from doing so. The best solution would be to use
    memset_s() (c11), though it's not readily available yet. This is the
    first step towards using memset_s() with a fallback for systems who
    don't have it. A better solution, is to put it in Eina, to prevent LTO
    completely. This will have to be done after the EFL release.
    Even this is not entirely safe though, but at least it protects us from
    some memory disclosure issues.
    
    This doesn't solve the fact that we may store a copy of the password in
    other places, like the input system. We need to address that too.
    
    Thanks to Matthew Garrett for pointing this out or Twitter.
---
 src/bin/e_auth.c            | 12 +++++-------
 src/bin/e_utils.c           | 16 ++++++++++++++++
 src/bin/e_utils.h           |  2 ++
 src/modules/lokker/lokker.c |  6 ++----
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/bin/e_auth.c b/src/bin/e_auth.c
index 7bda8ae..a85df6b 100644
--- a/src/bin/e_auth.c
+++ b/src/bin/e_auth.c
@@ -138,7 +138,7 @@ e_auth_begin(char *passwd)
    /* child */
    int pamerr;
    E_Auth da;
-   char *current_user, *p;
+   char *current_user;
    struct sigaction action;
 
    _e_auth_child_pid = fork();
@@ -158,8 +158,8 @@ e_auth_begin(char *passwd)
    eina_strlcpy(da.user, current_user, sizeof(da.user));
    eina_strlcpy(da.passwd, passwd, sizeof(da.passwd));
    /* security - null out passwd string once we are done with it */
-   for (p = passwd; *p; p++)
-     *p = 0;
+   e_util_memclear(passwd, strlen(passwd));
+
    da.pam.handle = NULL;
    da.pam.conv.conv = NULL;
    da.pam.conv.appdata_ptr = NULL;
@@ -173,10 +173,8 @@ e_auth_begin(char *passwd)
    pamerr = pam_authenticate(da.pam.handle, 0);
    pam_end(da.pam.handle, pamerr);
    /* security - null out passwd string once we are done with it */
-   memset(da.passwd, 0, sizeof(da.passwd));
-   /* break compiler optimization */
-   if (da.passwd[0] || da.passwd[3])
-     fprintf(stderr, "ACK!\n");
+   e_util_memclear(da.passwd, sizeof(da.passwd));
+
    if (pamerr == PAM_SUCCESS)
      {
         free(current_user);
diff --git a/src/bin/e_utils.c b/src/bin/e_utils.c
index 224e890..dc8b220 100644
--- a/src/bin/e_utils.c
+++ b/src/bin/e_utils.c
@@ -1423,3 +1423,19 @@ e_util_evas_objects_above_print_smart(Evas_Object *o)
           fprintf(stderr, "[%p] - %s(%s) %s\n", a, evas_object_type_get(a), 
evas_object_name_get(a), evas_object_visible_get(a) ? "VISIBLE" : "HIDDEN");
      }
 }
+
+/*
+ * NOTICE: This function should not be used by external modules!!!
+ *
+ * This function is just a hack to allow us to "securely" clear sensitive
+ * info until memset_s() is readily available, or at least we move this hack
+ * to Eina.
+ *
+ * This is going to work until link time optimizations are good enough.
+ * Hopefully by then, we'll be able to properly use memset_s().
+ */
+EAPI void
+e_util_memclear(void *s, size_t n)
+{
+   memset(s, 0, n);
+}
diff --git a/src/bin/e_utils.h b/src/bin/e_utils.h
index 1a99544..b17e2ff 100644
--- a/src/bin/e_utils.h
+++ b/src/bin/e_utils.h
@@ -63,6 +63,8 @@ EAPI void e_util_evas_objects_above_print_smart(Evas_Object 
*o);
 
 EAPI void e_util_string_list_free(Eina_List *l);
 
+EAPI void e_util_memclear(void *s, size_t n);
+
 static inline void
 e_util_pointer_center(const E_Client *ec)
 {
diff --git a/src/modules/lokker/lokker.c b/src/modules/lokker/lokker.c
index fdc7517..a9ecfbf 100644
--- a/src/modules/lokker/lokker.c
+++ b/src/modules/lokker/lokker.c
@@ -96,10 +96,8 @@ _text_passwd_update(void)
 static void
 _lokker_null(void)
 {
-   memset(edd->passwd, 0, sizeof(char) * PASSWD_LEN);
-   /* break compiler optimization */
-   if (edd->passwd[0] || edd->passwd[3])
-     fprintf(stderr, "ACK!\n");
+   e_util_memclear(edd->passwd, PASSWD_LEN);
+
    _text_passwd_update();
 }
 

-- 


Reply via email to