Tidied pgp memory allocations

prof.supp

@@ -27,28 +27,12 @@
   ...
 }
 
-# Ignore history module, needs to be rewritten
+# glib
 
 {
-  history_next
+  glib
   Memcheck:Leak
   ...
-  fun:history_next
+  fun:start_thread
-  ...
+  fun:clone
-}
-
-{
-  history_previous
-  Memcheck:Leak
-  ...
-  fun:history_previous
-  ...
-}
-
-{
-  history_append
-  Memcheck:Leak
-  ...
-  fun:history_append
-  ...
 }

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);

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;
+        GString *signed_gstr = g_string_new("");
-        result = _remove_header_footer(signed_str, PGP_SIGNATURE_FOOTER);
+        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);
+    char *cipher_str = gpgme_data_release_and_get_mem(cipher, &len);
-    if (cipher_str) {
-        result = _remove_header_footer(cipher_str, PGP_MESSAGE_FOOTER);
-    }
 
-    gpgme_free(cipher_str);
+    char *result = NULL;
-    gpgme_release(ctx);
+    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)
 {

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

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);
         }
     }

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) {}
+