fixed memory leaks

include/pcp.h

@@ -8,6 +8,7 @@ extern "C" {
 #include "pcp/config.h"
 #include "pcp/base85.h"
 #include "pcp/buffer.h"
+#include "pcp/config.h"
 #include "pcp/context.h"
 #include "pcp/crypto.h"
 #include "pcp/defines.h"

libpcp/buffer.c

@@ -329,6 +329,8 @@ size_t buffer_fd_read(Buffer *b, FILE *in, size_t len) {
   if(s > 0)
     buffer_add(b, data, len);
 
+  ucfree(data, len); /* FIXME: re-use data */
+
   return s;
 }
 

libpcp/crypto.c

@@ -276,14 +276,17 @@ size_t pcp_decrypt_stream(PCPCTX *ptx, Pcpstream *in, Pcpstream* out, pcp_key_t
     pcp_rec_t *rec = pcp_rec_new(reccipher, nrec * PCP_ASYM_RECIPIENT_SIZE, NULL, cur);
     size_t s = pcp_decrypt_stream_sym(ptx, in, out, symkey, rec);
     pcp_rec_free(rec);
+    ucfree(symkey, crypto_secretbox_KEYBYTES);
     return s;
   }
   else {
     size_t s = pcp_decrypt_stream_sym(ptx, in, out, symkey, NULL);
+    ucfree(symkey, crypto_secretbox_KEYBYTES);
     return s;
   }
 
  errdef1:
+  ucfree(symkey, crypto_secretbox_KEYBYTES);
   return 0;
 }
 
@@ -384,14 +387,12 @@ size_t pcp_encrypt_stream(PCPCTX *ptx, Pcpstream *in, Pcpstream *out, pcp_key_t
     goto errec1;
 
 
-  return out_size + sym_size;
-
-  
-
- errec1:
   memset(symkey, 0, crypto_secretbox_KEYBYTES);
   free(symkey);
   free(recipients_cipher);
+  return out_size + sym_size;
+
+ errec1:
 
   return 0;
 }

libpcp/key.c

@@ -26,7 +26,7 @@
 /*
  * AS of 16/01/2014 I'm using scrypt() instead of my crafted key
  * derivation function. However, I create a hash from the pcp_scrypt()
- * result anyway because I need a cure25519 secret.
+ * result anyway because I need a curve25519 secret.
  */
 byte *pcp_derivekey(PCPCTX *ptx, char *passphrase, byte *nonce) {
   byte *key = ucmalloc(crypto_secretbox_KEYBYTES);
@@ -43,9 +43,8 @@ byte *pcp_derivekey(PCPCTX *ptx, char *passphrase, byte *nonce) {
   key[31] &= 127;
   key[31] |= 64;
 
-  /* disabled, must be done outside 
+  /* done */
-     memset(passphrase, 0, plen); */
+  ucfree(scrypted, 64);
-
   return key;
 }
 
@@ -114,7 +113,10 @@ pcp_key_t * pcpkey_new () {
   memcpy (key->secret, cs, 32);
   memcpy (key->edpub, sp, 32);
   memcpy (key->edsecret, ss, 64);
-  memcpy (key->id, pcp_getkeyid(key), 17);
+
+  char *id = pcp_getkeyid(key);
+  memcpy (key->id, id, 17);
+  free(id);
 
   key->ctime = (long)time(0);
 
@@ -164,9 +166,10 @@ pcp_key_t *pcpkey_encrypt(PCPCTX *ptx, pcp_key_t *key, char *passphrase) {
   buffer_free(both);
   free(encryptkey);
 
-  if(es == 176) {
+  if(es == 176) { /* FIXME: calc! */
     /*  success */
     memcpy(key->encrypted, encrypted, 176);
+    ucfree(encrypted, es);
     arc4random_buf(key->secret, 32);
     arc4random_buf(key->edsecret, 64);
     arc4random_buf(key->mastersecret, 64);
@@ -176,7 +179,8 @@ pcp_key_t *pcpkey_encrypt(PCPCTX *ptx, pcp_key_t *key, char *passphrase) {
   }
   else {
     fatal(ptx, "failed to encrypt the secret key!\n");
-    free(key);
+    ucfree(encrypted, es);
+    ucfree(key, sizeof(pcp_key_t));
     return NULL;
   }
 
@@ -191,18 +195,19 @@ pcp_key_t *pcpkey_decrypt(PCPCTX *ptx, pcp_key_t *key, char *passphrase) {
   
   es = pcp_sodium_verify_mac(&decrypted, key->encrypted, 176, key->nonce, encryptkey);
 
-  memset(encryptkey, 0, 32);
+  ucfree(encryptkey, 32);
-  free(encryptkey);
 
   if(es == 0) {
     /*  success */
     memcpy(key->mastersecret, decrypted, 64);
     memcpy(key->edsecret, decrypted + 64, 64);    
     memcpy(key->secret, decrypted +128, 32);
+    ucfree(decrypted, 176);
   }
   else {
     fatal(ptx, "failed to decrypt the secret key (got %d, expected 32)!\n", es);
-    free(key);
+    ucfree(decrypted, 176);
+    ucfree(key, sizeof(pcp_key_t));
     return NULL;
   }
 

libpcp/vault.c

@@ -156,14 +156,14 @@ int pcpvault_addkey(PCPCTX *ptx, vault_t *vault, void *item, uint8_t type) {
   size_t itemsize;
 
   void *saveitem = NULL;
-  Buffer *blob = NULL;
+  Buffer *blob = buffer_new(PCP_RAW_KEYSIZE, "bs");
 
   if(type == PCP_KEY_TYPE_PUBLIC) {
     itemsize = PCP_RAW_PUBKEYSIZE;
     saveitem = ucmalloc(sizeof(pcp_pubkey_t));
     memcpy(saveitem, item, sizeof(pcp_pubkey_t));
     pubkey2be((pcp_pubkey_t *)item);
-    blob = pcp_keyblob(item, type);
+    pcp_pubkeyblob(blob, (pcp_pubkey_t *)item);
   }
   else if(type == PCP_KEYSIG_NATIVE || type == PCP_KEYSIG_NATIVE) {
     pcp_keysig_t *sk = (pcp_keysig_t *)item;
@@ -179,6 +179,7 @@ int pcpvault_addkey(PCPCTX *ptx, vault_t *vault, void *item, uint8_t type) {
     memcpy(saveitem, item, sizeof(pcp_key_t));
     key2be((pcp_key_t *)item);
     blob = pcp_keyblob(item, type);
+    pcp_seckeyblob(blob, (pcp_key_t *)item);
   }
 
 
@@ -274,7 +275,7 @@ void pcpvault_update_checksum(PCPCTX *ptx, vault_t *vault) {
 
 byte *pcpvault_create_checksum(PCPCTX *ptx) {
   pcp_key_t *k = NULL;
-  Buffer *blob = NULL;
+  Buffer *blob = buffer_new(PCP_RAW_KEYSIZE, "blob");;
   size_t datapos = 0;
 
   int numskeys = pcphash_count(ptx);
@@ -287,7 +288,7 @@ byte *pcpvault_create_checksum(PCPCTX *ptx) {
 
   pcphash_iterate(ptx, k) {
     key2be(k);
-    blob = pcp_keyblob(k, PCP_KEY_TYPE_SECRET);
+    pcp_seckeyblob(blob, (pcp_key_t *)k);
     memcpy(&data[datapos], buffer_get(blob), PCP_RAW_KEYSIZE);
     buffer_clear(blob);
     key2native(k);
@@ -298,7 +299,7 @@ byte *pcpvault_create_checksum(PCPCTX *ptx) {
   pcphash_iteratepub(ptx, p) {
     /* pcp_dumppubkey(p); */
     pubkey2be(p);
-    blob = pcp_keyblob(p, PCP_KEY_TYPE_PUBLIC);
+    pcp_pubkeyblob(blob, (pcp_pubkey_t *)p);
     memcpy(&data[datapos], buffer_get(blob), PCP_RAW_PUBKEYSIZE);
     buffer_clear(blob);
     pubkey2native(p);
@@ -371,6 +372,7 @@ int pcpvault_close(PCPCTX *ptx, vault_t *vault) {
       }
       fclose(vault->fd);
     }
+    free(vault->filename);
     free(vault);
     vault = NULL;
   }

src/encryption.c

@@ -106,6 +106,7 @@ int pcpdecrypt(char *id, int useid, char *infile, char *outfile, char *passwd, i
 	}
 
 	secret = pcpkey_decrypt(ptx, secret, passphrase);
+	ucfree(passphrase, strlen(passphrase));
 	if(secret == NULL)
 	  goto errde3;
 

src/keymgmt.c

@@ -173,13 +173,10 @@ char *pcp_normalize_id(char *keyid) {
 }
 
 pcp_key_t *pcp_find_primary_secret() {
-  pcp_key_t *key = NULL;
   pcp_key_t *k;
   pcphash_iterate(ptx, k) {
     if(k->type == PCP_KEY_TYPE_MAINSECRET) {
-      key = ucmalloc(sizeof(pcp_key_t));
+      return k;
-      memcpy(key, k, sizeof(pcp_key_t));
-      return key;
     }
   }
 
@@ -187,9 +184,7 @@ pcp_key_t *pcp_find_primary_secret() {
   int nkeys = pcphash_count(ptx);
   if(nkeys == 1) {
     pcphash_iterate(ptx, k) {
-      key = ucmalloc(sizeof(pcp_key_t));
+      return k;
-      memcpy(key, k, sizeof(pcp_key_t));
-      return key;
     }
   }
 

src/pcp.c

@@ -317,60 +317,75 @@ int main (int argc, char **argv)  {
        current mode and other given parameters */
     extra = ucmalloc(strlen(argv[0])+1);
     strncpy(extra, argv[0], strlen(argv[0])+1);
+    int useex = 0;
 
     switch (mode) {
     case PCP_MODE_DECRYPT:
-      if(infile == NULL)
+      if(infile == NULL) {
 	infile = extra;
+	useex = 1;
+      }
       break;
 
     case PCP_MODE_ENCRYPT:
-      if(infile == NULL)
+      if(infile == NULL) {
 	infile = extra;
+	useex = 1;
+      }
       else if(userec == 0 && useid == 0) {
 	userec = 1;
 	int i;
 	for (i=0; i<argc; i++) {
 	  p_add(&recipient, argv[i]);
 	}
-	free(extra);
       }
       break;
 
     case PCP_MODE_IMPORT: 
-      if(infile == NULL)
+      if(infile == NULL) {
 	infile = extra;
+	useex = 1;
+      }
       break;
 
     case PCP_MODE_EXPORT_SECRET:
     case PCP_MODE_EXPORT_PUBLIC:
-      if(outfile == NULL)
+      if(outfile == NULL) {
 	outfile = extra;
+	useex = 1;
+      }
       else if(useid == 0 && userec == 0) {
 	p_add(&recipient, extra);
+	useex = 1;
 	userec = 1;
       }
       break;
 
     case PCP_MODE_VERIFY:
-      if(infile == NULL)
+      if(infile == NULL) {
 	infile = extra;
+	useex = 1;
+      }
       else if (useid == 0) {
 	id = extra;
 	useid = 1;
+	useex = 1;
       }
       break;
 
     case PCP_MODE_SIGN:
-      if(infile == NULL)
+      if(infile == NULL) {
 	infile = extra;
-      else if(outfile == NULL && detach == 0)
+	useex = 1;
+      }
+      else if(outfile == NULL && detach == 0) {
 	outfile = extra;
+	useex = 1;
+      }
       break;
-
-    default:
-      free(extra); /* not used */
     }
+    if(useex)
+      free(extra);
   }
 
   /* check if there's some enviroment we could use */
@@ -393,8 +408,6 @@ int main (int argc, char **argv)  {
       switch (mode) {
       case  PCP_MODE_KEYGEN:
 	pcp_keygen(xpass);
-	if(xpass != NULL)
-	  free(xpass);
 	break;
 
       case PCP_MODE_LISTKEYS:
@@ -406,7 +419,6 @@ int main (int argc, char **argv)  {
 	  id = pcp_normalize_id(keyid);
 	  if(id != NULL) {
 	    pcp_exportsecret(id, useid, outfile, armor, xpass);
-	    free(id);
 	  }
 	}
 	else {
@@ -421,10 +433,6 @@ int main (int argc, char **argv)  {
 	    break;
 	}
 	pcp_exportpublic(id, xpass, outfile, exportformat, armor);
-	if(xpass != NULL)
-	  free(xpass);
-	if(recipient != NULL)
-	  free(recipient);
 	break;
 
       case PCP_MODE_IMPORT:
@@ -433,7 +441,6 @@ int main (int argc, char **argv)  {
 	else {
 	  if((in = fopen(infile, "rb")) == NULL) {
 	    fatal(ptx, "Could not open input file %s\n", infile);
-	    free(infile);
 	    break;
 	  }
 	}
@@ -445,7 +452,6 @@ int main (int argc, char **argv)  {
 	  id = pcp_normalize_id(keyid);
 	  if(id != NULL) {
 	    pcpdelete_key(id);
-	    free(id);
 	  }
 	}
 	else {
@@ -458,7 +464,6 @@ int main (int argc, char **argv)  {
 	  id = pcp_normalize_id(keyid);
 	  if(id != NULL) {
 	    pcpedit_key(id);
-	    free(id);
 	  }
 	}
 	else {
@@ -480,12 +485,6 @@ int main (int argc, char **argv)  {
 	  /*  -i and -r specified */
 	  fatal(ptx, "You can't specify both -i and -r, use either -i or -r!\n");
 	}
-	if(id != NULL)
-	  free(id);
-	if(xpass != NULL)
-	    free(xpass);
-	if(recipient != NULL)
-	  p_clean(recipient);
 
 	break;
 
@@ -494,14 +493,11 @@ int main (int argc, char **argv)  {
 	  id = pcp_normalize_id(keyid);
 	  if(id != NULL) {
 	    pcpdecrypt(id, useid, infile, outfile, xpass, signcrypt);
-	    free(id);
 	  }
 	}
 	else {
 	  pcpdecrypt(NULL, useid, infile, outfile, xpass, signcrypt);
 	}
-	if(xpass != NULL)
-	  free(xpass);
 	break;	
 
       case PCP_MODE_SIGN:
@@ -520,7 +516,6 @@ int main (int argc, char **argv)  {
 	  id = pcp_normalize_id(keyid);
 	  if(id != NULL) {
 	    pcpverify(infile, sigfile, id, detach);
-	    free(id);
 	  }
 	}
 	else {
@@ -538,7 +533,6 @@ int main (int argc, char **argv)  {
 	break;
       }
       pcpvault_close(ptx, vault);
-      free(vaultfile);
     }
   }
   else {
@@ -569,11 +563,9 @@ int main (int argc, char **argv)  {
 	  id = pcp_normalize_id(keyid);
 	  if(id != NULL) {
 	    pcptext_key(id);
-	    free(id);
 	  }
 	}
 	pcpvault_close(ptx, vault);
-	free(vaultfile);
       }
       break;
 
@@ -587,5 +579,21 @@ int main (int argc, char **argv)  {
   fatals_ifany(ptx);
   int e = ptx->pcp_exit;
   ptx_clean(ptx);
+
+  if(infile != NULL)
+    free(infile);
+  if(outfile != NULL)
+    free(outfile);
+  if(vaultfile != NULL)
+    free(vaultfile);
+  if(sigfile != NULL)
+    free(sigfile);
+  if(xpass != NULL)
+    ucfree(xpass, strlen(xpass));
+  if(recipient != NULL)
+    p_clean(recipient);
+  if(id != NULL)
+    free(id);
+
   return e;
 }

tests/cpptest.cpp

@@ -70,6 +70,7 @@ void test0() {
       if(strncmp(got, "HALLO", 5) != 0) {
 	throw pcp::exception(CA);
       }
+      free(got);
     }
     else
       throw pcp::exception(CA, "failed to decrypt");

tests/unittests.cfg

@@ -184,7 +184,7 @@ temporarily disabled
 */
 #
 # encryption tests
-<test check-crypto-alicia-init>
+<test check-crypto-alicia-2-bobby>
   # alicias part
   prepare = echo ${md5msg} > testmessage
   <test check-crypto-alicia-import-secret>
@@ -201,9 +201,7 @@ temporarily disabled
     cmd = $pcp -V va -e -i ${idbobby} -I testmessage -O testencrypted -x a
     expect = /for ${idbobby} successfully/
   </test>
-</test>
 
-<test check-crypto-bobby-init>
   # bobbys part
   <test check-crypto-bobby-import-secret>
     cmd = $pcp -V vb -K -I key-bobby-sec -x b