From b110da9a923c64be7ba33cac3212596c118ccd0d Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Wed, 10 Jul 2019 12:27:28 +0200 Subject: [PATCH 1/3] Fix various OMEMO memleaks --- src/omemo/omemo.c | 4 ++-- src/omemo/store.c | 11 ++++++++++- src/xmpp/connection.c | 6 ++++-- src/xmpp/stanza.c | 9 +++++++++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index 02107432..f82ad164 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -1363,14 +1363,14 @@ omemo_automatic_start(const char *const recipient) } else if (g_list_find_custom(account->omemo_disabled, recipient, (GCompareFunc)g_strcmp0)) { result = FALSE; } else { - return FALSE; + result = FALSE; } break; case PROF_OMEMOPOLICY_ALWAYS: if (g_list_find_custom(account->omemo_disabled, recipient, (GCompareFunc)g_strcmp0)) { result = FALSE; } else { - return TRUE; + result = TRUE; } break; } diff --git a/src/omemo/store.c b/src/omemo/store.c index 3e44be43..2efcae4c 100644 --- a/src/omemo/store.c +++ b/src/omemo/store.c @@ -38,10 +38,12 @@ #include "omemo/omemo.h" #include "omemo/store.h" +static void _g_hash_table_free(GHashTable *hash_table); + GHashTable * session_store_new(void) { - return g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL); + return g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)_g_hash_table_free); } GHashTable * @@ -441,3 +443,10 @@ load_sender_key(signal_buffer **record, signal_buffer **user_record, { return SG_SUCCESS; } + +static void +_g_hash_table_free(GHashTable *hash_table) +{ + g_hash_table_remove_all(hash_table); + g_hash_table_unref(hash_table); +} diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index 62b56666..46ae1ed8 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -290,6 +290,7 @@ connection_send_stanza(const char *const stanza) gboolean connection_supports(const char *const feature) { + gboolean ret = FALSE; GList *jids = g_hash_table_get_keys(conn.features_by_jid); GList *curr = jids; @@ -297,7 +298,8 @@ connection_supports(const char *const feature) char *jid = curr->data; GHashTable *features = g_hash_table_lookup(conn.features_by_jid, jid); if (features && g_hash_table_lookup(features, feature)) { - return TRUE; + ret = TRUE; + break; } curr = g_list_next(curr); @@ -305,7 +307,7 @@ connection_supports(const char *const feature) g_list_free(jids); - return FALSE; + return ret; } char* diff --git a/src/xmpp/stanza.c b/src/xmpp/stanza.c index affb6ff4..2801ab53 100644 --- a/src/xmpp/stanza.c +++ b/src/xmpp/stanza.c @@ -1869,6 +1869,15 @@ stanza_attach_publish_options(xmpp_ctx_t *const ctx, xmpp_stanza_t *const iq, co xmpp_stanza_t *pubsub = xmpp_stanza_get_child_by_ns(iq, STANZA_NS_PUBSUB); xmpp_stanza_add_child(pubsub, publish_options); + + xmpp_stanza_release(access_model_value_text); + xmpp_stanza_release(access_model_value); + xmpp_stanza_release(access_model); + xmpp_stanza_release(form_type_value_text); + xmpp_stanza_release(form_type_value); + xmpp_stanza_release(form_type); + xmpp_stanza_release(x); + xmpp_stanza_release(publish_options); } void From b7144d82febae18c235772bf8abae3c7684f3109 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Thu, 11 Jul 2019 07:43:11 +0320 Subject: [PATCH 2/3] Enable secure memory in gcrypt initialisation --- src/omemo/crypto.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/omemo/crypto.c b/src/omemo/crypto.c index 49be2003..81d7c922 100644 --- a/src/omemo/crypto.c +++ b/src/omemo/crypto.c @@ -47,6 +47,12 @@ omemo_crypto_init(void) return -1; } + gcry_control(GCRYCTL_SUSPEND_SECMEM_WARN); + + gcry_control(GCRYCTL_INIT_SECMEM, 16384, 0); + + gcry_control(GCRYCTL_RESUME_SECMEM_WARN); + gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0); return 0; From c22df13d9548ac99ed7a50f462227523ee15d8ed Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Thu, 11 Jul 2019 18:51:03 +0320 Subject: [PATCH 3/3] Dirty fix libgcrypt memleak --- prof.supp | 10 ++++++++++ src/omemo/crypto.c | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/prof.supp b/prof.supp index bc9ca50a..f894dc75 100644 --- a/prof.supp +++ b/prof.supp @@ -1176,3 +1176,13 @@ fun:SHA1_Update } + +# gcrypt initialization +{ + gcry_rngcsprng_randomize + Memcheck:Leak + fun:malloc + ... + fun:omemo_crypto_init + ... +} diff --git a/src/omemo/crypto.c b/src/omemo/crypto.c index 81d7c922..5a67c3b1 100644 --- a/src/omemo/crypto.c +++ b/src/omemo/crypto.c @@ -55,6 +55,11 @@ omemo_crypto_init(void) gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0); + /* Ask for a first random buffer to ensure CSPRNG is initialized. + * Thus we control the memleak produced by gcrypt initialization and we can + * suppress it without having false negatives */ + gcry_free(gcry_random_bytes_secure(1, GCRY_VERY_STRONG_RANDOM)); + return 0; }