From 2af5c151a02f3cde69e6e0b02a79f4f439f141e1 Mon Sep 17 00:00:00 2001 From: James Booth Date: Tue, 11 Aug 2015 01:00:23 +0100 Subject: [PATCH] Tidied pgp memory allocations --- prof.supp | 24 ++------ src/event/server_events.c | 2 + src/pgp/gpg.c | 109 ++++++++++++++++++++------------- src/pgp/gpg.h | 1 + src/xmpp/message.c | 4 +- tests/unittests/pgp/stub_gpg.c | 2 + 6 files changed, 78 insertions(+), 64 deletions(-) diff --git a/prof.supp b/prof.supp index fba137a2..e456551b 100644 --- a/prof.supp +++ b/prof.supp @@ -27,28 +27,12 @@ ... } -# Ignore history module, needs to be rewritten +# glib { - history_next + glib Memcheck:Leak ... - fun:history_next - ... -} - -{ - history_previous - Memcheck:Leak - ... - fun:history_previous - ... -} - -{ - history_append - Memcheck:Leak - ... - fun:history_append - ... + fun:start_thread + fun:clone } diff --git a/src/event/server_events.c b/src/event/server_events.c index ce667136..ee918bf1 100644 --- a/src/event/server_events.c +++ b/src/event/server_events.c @@ -218,6 +218,7 @@ sv_ev_incoming_message(char *barejid, char *resource, char *message, char *enc_m ui_incoming_msg(chatwin, resource, decrypted, NULL, new_win); chat_log_pgp_msg_in(barejid, decrypted); chatwin->enc_mode = PROF_ENC_PGP; + p_gpg_free_decrypted(decrypted); } else { ui_incoming_msg(chatwin, resource, message, NULL, new_win); chat_log_msg_in(barejid, message); @@ -267,6 +268,7 @@ sv_ev_incoming_message(char *barejid, char *resource, char *message, char *enc_m ui_incoming_msg(chatwin, resource, decrypted, NULL, new_win); chat_log_pgp_msg_in(barejid, decrypted); chatwin->enc_mode = PROF_ENC_PGP; + p_gpg_free_decrypted(decrypted); } else { ui_incoming_msg(chatwin, resource, message, NULL, new_win); chat_log_msg_in(barejid, message); diff --git a/src/pgp/gpg.c b/src/pgp/gpg.c index 020fd29a..39e743eb 100644 --- a/src/pgp/gpg.c +++ b/src/pgp/gpg.c @@ -132,6 +132,7 @@ p_gpg_on_connect(const char * const barejid) gpgme_ctx_t ctx; gpgme_error_t error = gpgme_new(&ctx); + if (error) { log_error("GPG: Failed to create gpgme context. %s %s", gpgme_strsource(error), gpgme_strerror(error)); g_strfreev(jids); @@ -157,7 +158,7 @@ p_gpg_on_connect(const char * const barejid) } g_hash_table_replace(fingerprints, strdup(jid), strdup(key->subkeys->fpr)); - gpgme_key_release(key); + gpgme_key_unref(key); } } @@ -196,6 +197,8 @@ p_gpg_addkey(const char * const jid, const char * const keyid) gpgme_key_t key = NULL; error = gpgme_get_key(ctx, keyid, &key, 1); + gpgme_release(ctx); + if (error || key == NULL) { log_error("GPG: Failed to get key. %s %s", gpgme_strsource(error), gpgme_strerror(error)); return FALSE; @@ -207,7 +210,7 @@ p_gpg_addkey(const char * const jid, const char * const keyid) // update in memory fingerprint list g_hash_table_replace(fingerprints, strdup(jid), strdup(key->subkeys->fpr)); - gpgme_key_release(key); + gpgme_key_unref(key); return TRUE; } @@ -216,11 +219,11 @@ GSList * p_gpg_list_keys(void) { gpgme_error_t error; - gpgme_ctx_t ctx; - gpgme_key_t key; GSList *result = NULL; + gpgme_ctx_t ctx; error = gpgme_new(&ctx); + if (error) { log_error("GPG: Could not list keys. %s %s", gpgme_strsource(error), gpgme_strerror(error)); return NULL; @@ -229,7 +232,9 @@ p_gpg_list_keys(void) error = gpgme_op_keylist_start(ctx, NULL, 1); if (error == GPG_ERR_NO_ERROR) { while (!error) { + gpgme_key_t key; error = gpgme_op_keylist_next(ctx, &key); + if (error) { break; } @@ -241,7 +246,7 @@ p_gpg_list_keys(void) result = g_slist_append(result, p_pgpkey); - gpgme_key_release(key); + gpgme_key_unref(key); } } else { log_error("GPG: Could not list keys. %s %s", gpgme_strsource(error), gpgme_strerror(error)); @@ -291,18 +296,24 @@ p_gpg_verify(const char * const barejid, const char *const sign) gpgme_ctx_t ctx; gpgme_error_t error = gpgme_new(&ctx); + if (error) { log_error("GPG: Failed to create gpgme context. %s %s", gpgme_strsource(error), gpgme_strerror(error)); return; } - gpgme_data_t sign_data; - gpgme_data_t plain_data; char *sign_with_header_footer = _add_header_footer(sign, PGP_SIGNATURE_HEADER, PGP_SIGNATURE_FOOTER); + gpgme_data_t sign_data; gpgme_data_new_from_mem(&sign_data, sign_with_header_footer, strlen(sign_with_header_footer), 1); + free(sign_with_header_footer); + + gpgme_data_t plain_data; gpgme_data_new(&plain_data); error = gpgme_op_verify(ctx, sign_data, NULL, plain_data); + gpgme_data_release(sign_data); + gpgme_data_release(plain_data); + if (error) { log_error("GPG: Failed to verify. %s %s", gpgme_strsource(error), gpgme_strerror(error)); gpgme_release(ctx); @@ -317,10 +328,7 @@ p_gpg_verify(const char * const barejid, const char *const sign) } } - gpgme_data_release(sign_data); - gpgme_data_release(plain_data); gpgme_release(ctx); - free(sign_with_header_footer); } char* @@ -335,55 +343,58 @@ p_gpg_sign(const char * const str, const char * const fp) gpgme_key_t key = NULL; error = gpgme_get_key(ctx, fp, &key, 1); + if (error || key == NULL) { log_error("GPG: Failed to get key. %s %s", gpgme_strsource(error), gpgme_strerror(error)); gpgme_release(ctx); - if (key) { - gpgme_key_unref(key); - } return NULL; } gpgme_signers_clear(ctx); error = gpgme_signers_add(ctx, key); gpgme_key_unref(key); + if (error) { log_error("GPG: Failed to load signer. %s %s", gpgme_strsource(error), gpgme_strerror(error)); gpgme_release(ctx); return NULL; } - gpgme_data_t str_data; - gpgme_data_t signed_data; char *str_or_empty = NULL; if (str) { str_or_empty = strdup(str); } else { str_or_empty = strdup(""); } + gpgme_data_t str_data; gpgme_data_new_from_mem(&str_data, str_or_empty, strlen(str_or_empty), 1); + free(str_or_empty); + + gpgme_data_t signed_data; gpgme_data_new(&signed_data); gpgme_set_armor(ctx,1); - error = gpgme_op_sign(ctx,str_data,signed_data,GPGME_SIG_MODE_DETACH); + error = gpgme_op_sign(ctx, str_data, signed_data, GPGME_SIG_MODE_DETACH); + gpgme_data_release(str_data); + gpgme_release(ctx); + if (error) { log_error("GPG: Failed to sign string. %s %s", gpgme_strsource(error), gpgme_strerror(error)); - gpgme_release(ctx); + gpgme_data_release(signed_data); return NULL; } char *result = NULL; - gpgme_data_release(str_data); size_t len = 0; char *signed_str = gpgme_data_release_and_get_mem(signed_data, &len); if (signed_str) { - signed_str[len] = 0; - result = _remove_header_footer(signed_str, PGP_SIGNATURE_FOOTER); + GString *signed_gstr = g_string_new(""); + g_string_append_len(signed_gstr, signed_str, len); + result = _remove_header_footer(signed_gstr->str, PGP_SIGNATURE_FOOTER); + g_string_free(signed_gstr, TRUE); + gpgme_free(signed_str); } - gpgme_free(signed_str); - gpgme_release(ctx); - free(str_or_empty); return result; } @@ -411,6 +422,7 @@ p_gpg_encrypt(const char * const barejid, const char * const message) gpgme_key_t key; error = gpgme_get_key(ctx, fp, &key, 0); + if (error || key == NULL) { log_error("GPG: Failed to get key. %s %s", gpgme_strsource(error), gpgme_strerror(error)); gpgme_release(ctx); @@ -420,29 +432,33 @@ p_gpg_encrypt(const char * const barejid, const char * const message) keys[0] = key; gpgme_data_t plain; - gpgme_data_t cipher; gpgme_data_new_from_mem(&plain, message, strlen(message), 1); + + gpgme_data_t cipher; gpgme_data_new(&cipher); gpgme_set_armor(ctx, 1); error = gpgme_op_encrypt(ctx, keys, GPGME_ENCRYPT_ALWAYS_TRUST, plain, cipher); + gpgme_data_release(plain); + gpgme_release(ctx); + gpgme_key_unref(key); + if (error) { log_error("GPG: Failed to encrypt message. %s %s", gpgme_strsource(error), gpgme_strerror(error)); - gpgme_release(ctx); return NULL; } - gpgme_data_release(plain); - char *cipher_str = NULL; - char *result = NULL; size_t len; - cipher_str = gpgme_data_release_and_get_mem(cipher, &len); - if (cipher_str) { - result = _remove_header_footer(cipher_str, PGP_MESSAGE_FOOTER); - } + char *cipher_str = gpgme_data_release_and_get_mem(cipher, &len); - gpgme_free(cipher_str); - gpgme_release(ctx); + char *result = NULL; + if (cipher_str) { + GString *cipher_gstr = g_string_new(""); + g_string_append_len(cipher_gstr, cipher_str, len); + result = _remove_header_footer(cipher_gstr->str, PGP_MESSAGE_FOOTER); + g_string_free(cipher_gstr, TRUE); + gpgme_free(cipher_str); + } return result; } @@ -450,29 +466,32 @@ p_gpg_encrypt(const char * const barejid, const char * const message) char * p_gpg_decrypt(const char * const barejid, const char * const cipher) { - char *cipher_with_headers = _add_header_footer(cipher, PGP_MESSAGE_HEADER, PGP_MESSAGE_FOOTER); - gpgme_ctx_t ctx; gpgme_error_t error = gpgme_new(&ctx); + if (error) { log_error("GPG: Failed to create gpgme context. %s %s", gpgme_strsource(error), gpgme_strerror(error)); return NULL; } - gpgme_data_t plain_data; + char *cipher_with_headers = _add_header_footer(cipher, PGP_MESSAGE_HEADER, PGP_MESSAGE_FOOTER); gpgme_data_t cipher_data; - gpgme_data_new_from_mem (&cipher_data, cipher_with_headers, strlen(cipher_with_headers), 1); + gpgme_data_new_from_mem(&cipher_data, cipher_with_headers, strlen(cipher_with_headers), 1); + free(cipher_with_headers); + + gpgme_data_t plain_data; gpgme_data_new(&plain_data); error = gpgme_op_decrypt(ctx, cipher_data, plain_data); + gpgme_data_release(cipher_data); + gpgme_release(ctx); + if (error) { log_error("GPG: Failed to encrypt message. %s %s", gpgme_strsource(error), gpgme_strerror(error)); - gpgme_release(ctx); + gpgme_data_release(plain_data); return NULL; } - gpgme_data_release(cipher_data); - size_t len = 0; char *plain_str = gpgme_data_release_and_get_mem(plain_data, &len); char *result = NULL; @@ -482,11 +501,15 @@ p_gpg_decrypt(const char * const barejid, const char * const cipher) } gpgme_free(plain_str); - gpgme_release(ctx); - return result; } +void +p_gpg_free_decrypted(char *decrypted) +{ + g_free(decrypted); +} + static char* _remove_header_footer(char *str, const char * const footer) { diff --git a/src/pgp/gpg.h b/src/pgp/gpg.h index 07c99465..77f14d1e 100644 --- a/src/pgp/gpg.h +++ b/src/pgp/gpg.h @@ -55,5 +55,6 @@ char* p_gpg_sign(const char * const str, const char * const fp); void p_gpg_verify(const char * const barejid, const char *const sign); char* p_gpg_encrypt(const char * const barejid, const char * const message); char* p_gpg_decrypt(const char * const barejid, const char * const cipher); +void p_gpg_free_decrypted(char *decrypted); #endif diff --git a/src/xmpp/message.c b/src/xmpp/message.c index b7ee1018..a46ead5c 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -174,6 +174,7 @@ message_send_chat_pgp(const char * const barejid, const char * const msg) } else { message = stanza_create_message(ctx, id, jid, STANZA_TYPE_CHAT, msg); } + account_free(account); #else message = stanza_create_message(ctx, id, jid, STANZA_TYPE_CHAT, msg); #endif @@ -772,6 +773,7 @@ _chat_handler(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza, void * con } // standard chat message, use jid without resource + xmpp_ctx_t *ctx = connection_get_ctx(); GDateTime *timestamp = stanza_get_delay(stanza); if (body) { char *message = xmpp_stanza_get_text(body); @@ -785,11 +787,11 @@ _chat_handler(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza, void * con enc_message = xmpp_stanza_get_text(x); } sv_ev_incoming_message(jid->barejid, jid->resourcepart, message, enc_message); + xmpp_free(ctx, enc_message); } _receipt_request_handler(stanza); - xmpp_ctx_t *ctx = connection_get_ctx(); xmpp_free(ctx, message); } } diff --git a/tests/unittests/pgp/stub_gpg.c b/tests/unittests/pgp/stub_gpg.c index 671b2092..f8e05d57 100644 --- a/tests/unittests/pgp/stub_gpg.c +++ b/tests/unittests/pgp/stub_gpg.c @@ -47,3 +47,5 @@ gboolean p_gpg_addkey(const char * const jid, const char * const keyid) return TRUE; } +void p_gpg_free_decrypted(char *decrypted) {} +