diff options
author | Christophe Fergeau <cfergeau@redhat.com> | 2014-09-21 18:14:20 +0200 |
---|---|---|
committer | Stef Walter <stefw@gnome.org> | 2014-11-13 21:44:23 +0100 |
commit | dc5a7dc4e5690e258a90d8ddfc39e17c1f8d4938 (patch) | |
tree | 064914b472c01e6716d4be58073b82be87ca5bee | |
parent | abf3473a9a14784a76911ef35c0916d462d55402 (diff) |
Don't leak password in login_prompt_do_{specific, user)
login_prompt_do_specific() and login_prompt_do_user() both
set GkmWrapPrompt::prompt_data to either memory which must be freed
with egg_secure_memory_strfree (through a call to
auto_unlock_lookup_*) or to const memory which must not be freed
(through a call to gkm_wrap_prompt_request_password).
These methods currently assume the password memory does not need
to be freed, which leads to this leak in test-login-auto (line numbers
from 3.13.91-2-g45bb5be):
==2190== 5 bytes in 1 blocks are definitely lost in loss record 17 of 1,294
==2190== at 0x40F8E4: egg_secure_alloc_full (egg-secure-memory.c:1056)
==2190== by 0x417F5E: egg_secure_alloc (gkm-wrap-login.c:42)
==2190== by 0x419157: gkm_wrap_login_lookup_secret (gkm-wrap-login.c:409)
==2190== by 0x407E8D: auto_unlock_lookup_object (gkm-wrap-prompt.c:198)
==2190== by 0x40B9B0: login_prompt_do_specific (gkm-wrap-prompt.c:1453)
==2190== by 0x40C13A: gkm_wrap_prompt_do_login (gkm-wrap-prompt.c:1591)
==2190== by 0x406384: auth_C_Login (gkm-wrap-layer.c:706)
==2190== by 0x40472A: test_specific (test-login-auto.c:156)
==2190== by 0x5E0A27A: test_case_run (gtestutils.c:2059)
==2190== by 0x5E0A602: g_test_run_suite_internal (gtestutils.c:2120)
==2190== by 0x5E0A6C4: g_test_run_suite_internal (gtestutils.c:2131)
==2190== by 0x5E0A6C4: g_test_run_suite_internal (gtestutils.c:2131)
==2190== by 0x5E0A847: g_test_run_suite (gtestutils.c:2184)
==2190== by 0x5E09551: g_test_run (gtestutils.c:1488)
==2190== by 0x410851: testing_thread (egg-testing.c:142)
==2190== by 0x5E0D2F4: g_thread_proxy (gthread.c:764)
==2190== by 0x3B7AE07F34: start_thread (pthread_create.c:309)
==2190== by 0x3B7AAF4C3C: clone (clone.S:111)
https://bugzilla.gnome.org/show_bug.cgi?id=738508
-rw-r--r-- | pkcs11/wrap-layer/gkm-wrap-prompt.c | 62 |
1 files changed, 44 insertions, 18 deletions
diff --git a/pkcs11/wrap-layer/gkm-wrap-prompt.c b/pkcs11/wrap-layer/gkm-wrap-prompt.c index 81a27055..5b86d1d7 100644 --- a/pkcs11/wrap-layer/gkm-wrap-prompt.c +++ b/pkcs11/wrap-layer/gkm-wrap-prompt.c @@ -904,14 +904,30 @@ gkm_wrap_prompt_init (GkmWrapPrompt *self) } static void -gkm_wrap_prompt_finalize (GObject *obj) +gkm_wrap_prompt_clear_prompt_data (GkmWrapPrompt *self) { - GkmWrapPrompt *self = GKM_WRAP_PROMPT (obj); - if (self->destroy_data && self->prompt_data) (self->destroy_data) (self->prompt_data); self->destroy_data = NULL; self->prompt_data = NULL; +} + +static void +gkm_wrap_prompt_set_prompt_data (GkmWrapPrompt *self, + gpointer prompt_data, + GDestroyNotify destroy_data) +{ + gkm_wrap_prompt_clear_prompt_data (self); + self->destroy_data = destroy_data; + self->prompt_data = prompt_data; +} + +static void +gkm_wrap_prompt_finalize (GObject *obj) +{ + GkmWrapPrompt *self = GKM_WRAP_PROMPT (obj); + + gkm_wrap_prompt_clear_prompt_data (self); while (!g_queue_is_empty(&self->pool)) g_free (g_queue_pop_head (&self->pool)); @@ -977,8 +993,8 @@ gkm_wrap_prompt_for_credential (CK_FUNCTION_LIST_PTR module, CK_SESSION_HANDLE s NULL); /* Build up the prompt */ - self->prompt_data = data = g_slice_new0 (CredentialPrompt); - self->destroy_data = credential_prompt_free; + data = g_slice_new0 (CredentialPrompt); + gkm_wrap_prompt_set_prompt_data (self, data, credential_prompt_free); self->module = module; self->session = session; self->object = object; @@ -1173,7 +1189,7 @@ gkm_wrap_prompt_do_init_pin (GkmWrapPrompt *self, CK_RV last_result, CK_UTF8CHAR_PTR *pin, CK_ULONG *n_pin) { CK_TOKEN_INFO tinfo; - const gchar *password; + gchar *password; g_assert (GKM_IS_WRAP_PROMPT (self)); g_assert (self->module); @@ -1185,11 +1201,12 @@ gkm_wrap_prompt_do_init_pin (GkmWrapPrompt *self, CK_RV last_result, setup_init_token (self, &tinfo); - password = gkm_wrap_prompt_request_password (self); + password = (char *)gkm_wrap_prompt_request_password (self); if (password == NULL) return FALSE; - self->prompt_data = (gpointer)password; + g_assert (self->destroy_data == NULL); + gkm_wrap_prompt_set_prompt_data (self, password, NULL); *pin = (gpointer)password; *n_pin = strlen (password); return TRUE; @@ -1314,8 +1331,9 @@ gkm_wrap_prompt_for_set_pin (CK_FUNCTION_LIST_PTR module, CK_SESSION_HANDLE sess /* Build up the prompt */ self->module = module; self->session = session; - self->destroy_data = set_pin_prompt_free; - self->prompt_data = g_slice_new0 (SetPinPrompt); + gkm_wrap_prompt_set_prompt_data (self, + g_slice_new0 (SetPinPrompt), + set_pin_prompt_free); return self; } @@ -1438,7 +1456,7 @@ static gboolean login_prompt_do_specific (GkmWrapPrompt *self, CK_RV last_result, CK_UTF8CHAR_PTR *pin, CK_ULONG *n_pin) { - const gchar *password = NULL; + gchar *password = NULL; CK_ATTRIBUTE_PTR attrs; CK_ULONG n_attrs; @@ -1451,6 +1469,9 @@ login_prompt_do_specific (GkmWrapPrompt *self, CK_RV last_result, if (self->iteration == 0) { ++(self->iteration); password = auto_unlock_lookup_object (attrs, n_attrs); + if (password) + gkm_wrap_prompt_set_prompt_data (self, password, + (GDestroyNotify)egg_secure_strfree); } else if (self->iteration == 1 && last_result == CKR_PIN_INCORRECT) { auto_unlock_remove_object (attrs, n_attrs); @@ -1459,12 +1480,12 @@ login_prompt_do_specific (GkmWrapPrompt *self, CK_RV last_result, if (!password) { setup_unlock_prompt (self, attrs, n_attrs, self->iteration == 1); - password = gkm_wrap_prompt_request_password (self); + password = (char *)gkm_wrap_prompt_request_password (self); if (password == NULL) return FALSE; + gkm_wrap_prompt_set_prompt_data (self, password, NULL); } - self->prompt_data = (gpointer)password; *pin = (guchar *)password; *n_pin = strlen (password); return TRUE; @@ -1477,7 +1498,8 @@ login_prompt_done_specific (GkmWrapPrompt *self, CK_RV call_result) CK_ULONG n_attrs; g_assert (GKM_IS_WRAP_PROMPT (self)); - g_assert (self->destroy_data == NULL); + g_assert (self->destroy_data == NULL || + self->destroy_data == (GDestroyNotify)egg_secure_strfree); /* Possibly save away auto unlock */ if (call_result == CKR_OK && auto_unlock_should_attach (self)) { @@ -1510,7 +1532,7 @@ login_prompt_do_user (GkmWrapPrompt *self, CK_RV last_result, CK_UTF8CHAR_PTR *pin, CK_ULONG *n_pin) { CK_TOKEN_INFO tinfo; - const gchar *password = NULL; + gchar *password = NULL; g_assert (GKM_IS_WRAP_PROMPT (self)); g_assert (self->module); @@ -1523,6 +1545,9 @@ login_prompt_do_user (GkmWrapPrompt *self, CK_RV last_result, if (self->iteration == 0) { ++(self->iteration); password = auto_unlock_lookup_token (&tinfo); + if (password) + gkm_wrap_prompt_set_prompt_data (self, password, + (GDestroyNotify)egg_secure_strfree); } else if (self->iteration == 1 && last_result == CKR_PIN_INCORRECT) { auto_unlock_remove_token (&tinfo); @@ -1531,12 +1556,12 @@ login_prompt_do_user (GkmWrapPrompt *self, CK_RV last_result, if (!password) { setup_unlock_token (self, &tinfo); - password = gkm_wrap_prompt_request_password (self); + password = (char *)gkm_wrap_prompt_request_password (self); if (password == NULL) return FALSE; + gkm_wrap_prompt_set_prompt_data (self, password, NULL); } - self->prompt_data = (gpointer)password; *pin = (guchar *)password; *n_pin = strlen (password); return TRUE; @@ -1548,7 +1573,8 @@ login_prompt_done_user (GkmWrapPrompt *self, CK_RV call_result) CK_TOKEN_INFO tinfo; g_assert (GKM_IS_WRAP_PROMPT (self)); - g_assert (self->destroy_data == NULL); + g_assert (self->destroy_data == NULL || + self->destroy_data == (GDestroyNotify)egg_secure_strfree); /* Save the options, and possibly auto unlock */ if (call_result == CKR_OK && auto_unlock_should_attach (self)) { |