Hi,

I discovered some crashes when using mcabber with PGP encryption. I've
created a patch that fixes these issues for me.

It is only tested on OpenBSD/i386 5.6.

An issue including the patch has also been created on bitbucket.

https://bitbucket.org/McKael/mcabber-crew/issue/134/crashes-while-using-pgp-encryption

Sven


--- mcabber/pgp.c.orig  Thu May 08 18:38:40 2014 +0200
+++ mcabber/pgp.c       Sun Feb 01 20:55:22 2015 +0100
@@ -209,11 +209,18 @@
 
   gpgme_set_protocol(ctx, GPGME_PROTOCOL_OpenPGP);
 
-  // Surround the given data with the prefix & suffix
-  data = g_new(char, sizeof(prefix) + sizeof(suffix) + strlen(gpg_data));
-  strcpy(data, prefix);
-  strcat(data, gpg_data);
-  strcat(data, suffix);
+  // Surround the given data with the prefix & suffix (prefix + suffix are 
0-terminated)
+  // so there is enough space
+  const int buf_len = sizeof(prefix) + sizeof(suffix) + strlen(gpg_data) + 1;
+  data = g_new(char, buf_len);
+  const int written_data = snprintf(data, buf_len, "%s%s%s", prefix, gpg_data, 
suffix);
+  if ((written_data == -1) || (written_data > buf_len)) {
+      scr_LogPrint(LPRINT_LOGNORM|LPRINT_NOTUTF8,
+                   "GPGME verification failed: could not construct buffer. %d 
!= %d",
+                   written_data, buf_len);
+      g_free(data);
+      return NULL;
+  }
 
   err = gpgme_data_new_from_mem(&data_sign, data, strlen(data), 0);
   if (!err) {
@@ -223,19 +230,26 @@
       if (!err) {
         gpgme_verify_result_t vr = gpgme_op_verify_result(ctx);
         if (vr && vr->signatures) {
-          char *r = vr->signatures->fpr;
-          // Found the fingerprint.  Let's try to get the key id.
-          if (!gpgme_get_key(ctx, r, &key, 0) && key) {
-            r = key->subkeys->keyid;
-            gpgme_key_release(key);
-          }
-          // r is a static variable, let's copy it.
-          verified_key = g_strdup(r);
-          *sigsum = vr->signatures->summary;
-          // For some reason summary could be 0 when status is 0 too,
-          // which means the signature is valid...
-          if (!*sigsum && !vr->signatures->status)
-            *sigsum = GPGME_SIGSUM_GREEN;
+            gpgme_signature_t s = NULL;
+            // check all signatures and stop if the first could be verified
+            for(s = vr->signatures;
+                (s != NULL) && (verified_key != NULL);
+                s = s->next) {
+                // Found the fingerprint.  Let's try to get the key id.
+                if (NULL != s->fpr) {
+                    if (!gpgme_get_key(ctx, s->fpr, &key, 0)) {
+                        if (key) {
+                            verified_key = g_strdup(key->subkeys->keyid);
+                            gpgme_key_release(key);
+                        }
+                    }
+                }
+                *sigsum = s->summary;
+                // For some reason summary could be 0 when status is 0 too,
+                // which means the signature is valid...
+                if ((!*sigsum) && (!s->status))
+                    *sigsum = GPGME_SIGSUM_GREEN;
+            }
         }
       }
       gpgme_data_release(data_text);
@@ -348,11 +362,17 @@
   if (!(p && strchr(p, ':')))
     gpgme_set_passphrase_cb(ctx, passphrase_cb, 0);
 
-  // Surround the given data with the prefix & suffix
-  data = g_new(char, sizeof(prefix) + sizeof(suffix) + strlen(gpg_data));
-  strcpy(data, prefix);
-  strcat(data, gpg_data);
-  strcat(data, suffix);
+  // Surround the given data with the prefix & suffix (prefix + suffix are 
0-terminated)
+  // so there is enough space
+  const int buf_len = sizeof(prefix) + sizeof(suffix) + strlen(gpg_data) + 1;
+  data = g_new(char, buf_len);
+  const int written_data = snprintf(data, buf_len, "%s%s%s", prefix, gpg_data, 
suffix);
+  if ((written_data == -1) || (written_data > buf_len)) {
+      scr_LogPrint(LPRINT_LOGNORM|LPRINT_NOTUTF8,
+                   "GPGME verification failed: could not construct buffer.");
+      g_free(data);
+      return NULL;
+  }
 
   err = gpgme_data_new_from_mem(&in, data, strlen(data), 0);
   if (!err) {
@@ -388,8 +408,9 @@
 {
   gpgme_ctx_t ctx;
   gpgme_data_t in, out;
-  char *encrypted_data = NULL, *edata;
-  size_t nread;
+  char *encrypted_data = NULL, *edata = NULL;
+  char *copy_encrypted_data = NULL;
+  size_t nread = 0;
   gpgme_key_t key;
   gpgme_error_t err;
 
@@ -415,8 +436,14 @@
       err = gpgme_data_new(&out);
       if (!err) {
         err = gpgme_op_encrypt(ctx, keys, GPGME_ENCRYPT_ALWAYS_TRUST, in, out);
-        if (!err)
+        if (!err) {
           encrypted_data = gpgme_data_release_and_get_mem(out, &nread);
+          if (encrypted_data) {
+              // We need to add a trailing NULL
+              copy_encrypted_data = g_strndup(encrypted_data, nread);
+              free(encrypted_data);
+          }
+        }
         else
           gpgme_data_release(out);
       }
@@ -431,9 +458,9 @@
     scr_LogPrint(LPRINT_LOGNORM|LPRINT_NOTUTF8,
                  "GPGME encryption error: %s", gpgme_strerror(err));
   gpgme_release(ctx);
-  edata = strip_header_footer(encrypted_data);
-  if (encrypted_data)
-    free(encrypted_data);
+  edata = strip_header_footer(copy_encrypted_data);
+  g_free(copy_encrypted_data);
+
   return edata;
 }
 
--- mcabber/xmpp.c.orig Thu May 08 18:38:40 2014 +0200
+++ mcabber/xmpp.c      Sun Feb 01 20:55:22 2015 +0100
@@ -822,6 +822,7 @@
     break;
   case LM_SSL_STATUS_CERT_FINGERPRINT_MISMATCH: {
     char fpr[49];
+    memset(fpr, 0, sizeof(fpr));
     fingerprint_to_hex((const unsigned char*)lm_ssl_get_fingerprint(ssl),
                        fpr);
     scr_LogPrint(LPRINT_LOGNORM,
@@ -1811,6 +1812,8 @@
   LmMessageHandler *handler;
   GError *error = NULL;
 
+  memset(fpr, 0, sizeof(fpr));
+
   xmpp_disconnect();
 
   servername  = settings_opt_get("server");

Reply via email to