From 3c8c127e0e629f4075287ad33e54d5c51b24eddb Mon Sep 17 00:00:00 2001
From: Christian Grothoff <christian@grothoff.org>
Date: Tue, 22 Sep 2015 10:00:01 +0200
Subject: fixing use of wrong array length (bad), and misc leaks

---
 src/mint/taler-mint-httpd_refresh.c   | 94 +++++++++++------------------------
 src/mint/taler-mint-httpd_responses.c |  2 +-
 src/mint/taler-mint-httpd_wire.c      |  1 +
 src/mint/test_taler_mint_httpd_afl.sh |  8 ++-
 4 files changed, 38 insertions(+), 67 deletions(-)

(limited to 'src')

diff --git a/src/mint/taler-mint-httpd_refresh.c b/src/mint/taler-mint-httpd_refresh.c
index 29fd4baea..4d89fe892 100644
--- a/src/mint/taler-mint-httpd_refresh.c
+++ b/src/mint/taler-mint-httpd_refresh.c
@@ -417,10 +417,8 @@ handle_refresh_melt_json (struct MHD_Connection *connection,
                                    &denom_pubs[i].rsa_public_key);
     if (GNUNET_OK != res)
     {
-      for (j=0;j<i;j++)
-        GNUNET_CRYPTO_rsa_public_key_free (denom_pubs[j].rsa_public_key);
-      GNUNET_free (denom_pubs);
-      return res;
+      res = (GNUNET_NO == res) ? MHD_YES : MHD_NO;
+      goto cleanup_denoms;
     }
     buf_size = GNUNET_CRYPTO_rsa_public_key_encode (denom_pubs[i].rsa_public_key,
                                                     &buf);
@@ -444,16 +442,8 @@ handle_refresh_melt_json (struct MHD_Connection *connection,
     if (GNUNET_OK != res)
     {
       GNUNET_break_op (0);
-      for (j=0;j<i;j++)
-      {
-        GNUNET_CRYPTO_rsa_public_key_free (coin_melt_details[j].coin_info.denom_pub.rsa_public_key);
-        GNUNET_CRYPTO_rsa_signature_free (coin_melt_details[j].coin_info.denom_sig.rsa_signature);
-      }
-      for (j=0;j<num_new_denoms;j++)
-        GNUNET_CRYPTO_rsa_public_key_free (denom_pubs[j].rsa_public_key);
-      GNUNET_free (coin_melt_details);
-      GNUNET_free (denom_pubs);
-      return (GNUNET_NO == res) ? MHD_YES : MHD_NO;
+      res = (GNUNET_NO == res) ? MHD_YES : MHD_NO;
+      goto cleanup_melt_details;
     }
     /* Check that the client does not try to melt the same coin twice
        into the same session! */
@@ -464,17 +454,9 @@ handle_refresh_melt_json (struct MHD_Connection *connection,
                        sizeof (struct TALER_CoinSpendPublicKeyP)))
       {
         GNUNET_break_op (0);
-        for (j=0;j<i;j++)
-        {
-          GNUNET_CRYPTO_rsa_public_key_free (coin_melt_details[j].coin_info.denom_pub.rsa_public_key);
-          GNUNET_CRYPTO_rsa_signature_free (coin_melt_details[j].coin_info.denom_sig.rsa_signature);
-        }
-        for (j=0;j<num_new_denoms;j++)
-          GNUNET_CRYPTO_rsa_public_key_free (denom_pubs[j].rsa_public_key);
-        GNUNET_free (coin_melt_details);
-        GNUNET_free (denom_pubs);
-        return TMH_RESPONSE_reply_external_error (connection,
-                                                  "melting same coin twice in same session is not allowed");
+        res = TMH_RESPONSE_reply_external_error (connection,
+                                                 "melting same coin twice in same session is not allowed");
+        goto cleanup_melt_details;
       }
     }
     TALER_amount_hton (&melt_amount,
@@ -512,11 +494,8 @@ handle_refresh_melt_json (struct MHD_Connection *connection,
       if (GNUNET_OK != res)
       {
         GNUNET_break_op (0);
-        GNUNET_CRYPTO_hash_context_abort (hash_context);
-        free_commit_coins (commit_coin,
-                           TALER_CNC_KAPPA,
-                           num_newcoins);
-        return (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
+        res = (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
+        goto cleanup;
       }
 
       GNUNET_CRYPTO_hash_context_read (hash_context,
@@ -532,11 +511,8 @@ handle_refresh_melt_json (struct MHD_Connection *connection,
       if (GNUNET_OK != res)
       {
         GNUNET_break_op (0);
-        GNUNET_CRYPTO_hash_context_abort (hash_context);
-        free_commit_coins (commit_coin,
-                           TALER_CNC_KAPPA,
-                           num_newcoins);
-        return (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
+        res = (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
+        goto cleanup;
       }
       rcc->refresh_link
         = TALER_refresh_link_encrypted_decode (link_enc,
@@ -563,19 +539,11 @@ handle_refresh_melt_json (struct MHD_Connection *connection,
                                      TMH_PARSE_JNC_RET_DATA,
                                      &rcl->transfer_pub,
                                      sizeof (struct TALER_TransferPublicKeyP));
-
       if (GNUNET_OK != res)
       {
         GNUNET_break_op (0);
-        GNUNET_break (GNUNET_SYSERR != res);
-        GNUNET_CRYPTO_hash_context_abort (hash_context);
-        free_commit_coins (commit_coin,
-                           TALER_CNC_KAPPA,
-                           num_newcoins);
-        free_commit_links (commit_link,
-                           TALER_CNC_KAPPA,
-                           num_oldcoins);
-        return (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
+        res = (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
+        goto cleanup;
       }
       res = TMH_PARSE_navigate_json (connection,
                                      secret_encs,
@@ -584,30 +552,20 @@ handle_refresh_melt_json (struct MHD_Connection *connection,
                                      TMH_PARSE_JNC_RET_DATA,
                                      &rcl->shared_secret_enc,
                                      sizeof (struct TALER_EncryptedLinkSecretP));
-
       if (GNUNET_OK != res)
       {
         GNUNET_break_op (0);
-        GNUNET_break (GNUNET_SYSERR != res);
-        GNUNET_CRYPTO_hash_context_abort (hash_context);
-        free_commit_coins (commit_coin,
-                           TALER_CNC_KAPPA,
-                           num_newcoins);
-        free_commit_links (commit_link,
-                           TALER_CNC_KAPPA,
-                           num_oldcoins);
-        return (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
+        res = (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
+        goto cleanup;
       }
-
       GNUNET_CRYPTO_hash_context_read (hash_context,
                                        rcl,
                                        sizeof (struct TALER_RefreshCommitLinkP));
     }
-
   }
   GNUNET_CRYPTO_hash_context_finish (hash_context,
                                      &session_hash);
-
+  hash_context = NULL;
   for (i=0;i<coin_count;i++)
   {
     /* verify signatures on coins to melt */
@@ -638,15 +596,22 @@ handle_refresh_melt_json (struct MHD_Connection *connection,
   free_commit_links (commit_link,
                      TALER_CNC_KAPPA,
                      num_oldcoins);
+ cleanup_melt_details:
   for (j=0;j<coin_count;j++)
   {
-    GNUNET_CRYPTO_rsa_public_key_free (coin_melt_details[j].coin_info.denom_pub.rsa_public_key);
-    GNUNET_CRYPTO_rsa_signature_free (coin_melt_details[j].coin_info.denom_sig.rsa_signature);
+    if (NULL != coin_melt_details[j].coin_info.denom_pub.rsa_public_key)
+      GNUNET_CRYPTO_rsa_public_key_free (coin_melt_details[j].coin_info.denom_pub.rsa_public_key);
+    if (NULL != coin_melt_details[j].coin_info.denom_sig.rsa_signature)
+      GNUNET_CRYPTO_rsa_signature_free (coin_melt_details[j].coin_info.denom_sig.rsa_signature);
   }
-  for (j=0;j<num_new_denoms;j++)
-    GNUNET_CRYPTO_rsa_public_key_free (denom_pubs[j].rsa_public_key);
   GNUNET_free (coin_melt_details);
+ cleanup_denoms:
+  for (j=0;j<num_new_denoms;j++)
+    if (NULL != denom_pubs[j].rsa_public_key)
+      GNUNET_CRYPTO_rsa_public_key_free (denom_pubs[j].rsa_public_key);
   GNUNET_free (denom_pubs);
+  if (NULL != hash_context)
+    GNUNET_CRYPTO_hash_context_abort (hash_context);
   return res;
 }
 
@@ -736,6 +701,7 @@ TMH_REFRESH_handler_refresh_melt (struct TMH_RequestHandler *rh,
     return (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
   }
   num_newcoins = json_array_size (coin_detail);
+  json_decref (coin_detail);
   res = TMH_PARSE_navigate_json (connection,
                                  transfer_pubs,
                                  TMH_PARSE_JNC_INDEX, (int) 0,
@@ -748,7 +714,7 @@ TMH_REFRESH_handler_refresh_melt (struct TMH_RequestHandler *rh,
     return (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
   }
   num_oldcoins = json_array_size (coin_detail);
-
+  json_decref (coin_detail);
   res = handle_refresh_melt_json (connection,
                                   new_denoms,
                                   melt_coins,
@@ -758,7 +724,6 @@ TMH_REFRESH_handler_refresh_melt (struct TMH_RequestHandler *rh,
                                   num_newcoins,
                                   coin_evs,
                                   link_encs);
-
   TMH_PARSE_release_data (spec);
   return res;
 }
@@ -899,6 +864,7 @@ TMH_REFRESH_handler_refresh_reveal (struct TMH_RequestHandler *rh,
     return (GNUNET_SYSERR == res) ? MHD_NO : MHD_YES;
   }
   num_oldcoins = json_array_size (reveal_detail);
+  json_decref (reveal_detail);
   res = handle_refresh_reveal_json (connection,
                                     &session_hash,
                                     num_oldcoins,
diff --git a/src/mint/taler-mint-httpd_responses.c b/src/mint/taler-mint-httpd_responses.c
index 50a0553e6..a018f68d3 100644
--- a/src/mint/taler-mint-httpd_responses.c
+++ b/src/mint/taler-mint-httpd_responses.c
@@ -947,7 +947,7 @@ TMH_RESPONSE_reply_refresh_reveal_missmatch (struct MHD_Connection *connection,
     json_array_append_new (info_commit,
                            info_commit_k);
     info_link_k = json_array ();
-    for (i=0;i<mc->num_newcoins;i++)
+    for (i=0;i<mc->num_oldcoins;i++)
     {
       const struct TALER_RefreshCommitLinkP *cl;
       json_t *cl_json;
diff --git a/src/mint/taler-mint-httpd_wire.c b/src/mint/taler-mint-httpd_wire.c
index 9c00b5d43..1eb3f6bef 100644
--- a/src/mint/taler-mint-httpd_wire.c
+++ b/src/mint/taler-mint-httpd_wire.c
@@ -127,6 +127,7 @@ TMH_WIRE_handler_wire_test (struct TMH_RequestHandler *rh,
 					     &wire_test_redirect))
   {
     /* oopsie, configuration error */
+    MHD_destroy_response (response);
     return TMH_RESPONSE_reply_internal_error (connection,
 					      "REDIRECT_URL not configured");
   }
diff --git a/src/mint/test_taler_mint_httpd_afl.sh b/src/mint/test_taler_mint_httpd_afl.sh
index bbcf8edbf..d2c40c216 100755
--- a/src/mint/test_taler_mint_httpd_afl.sh
+++ b/src/mint/test_taler_mint_httpd_afl.sh
@@ -21,17 +21,21 @@
 #
 # We read the JSON snippets from afl-tests/
 #
+PREFIX=
+# Uncomment this line to run with valgrind...
+PREFIX="valgrind --leak-check=yes --log-file=valgrind.%p"
 # Setup keys.
 taler-mint-keyup -d test-mint-home -m test-mint-home/master.priv
 # Setup database (just to be sure)
 taler-mint-dbinit -d test-mint-home &> /dev/null || true
 # Only log hard errors, we expect lots of warnings...
-export GNUNET_FORCE_LOG="taler-mint-httpd;;;;ERROR/libmicrohttpd;;;;ERROR/"
+export GNUNET_FORCE_LOG="taler-mint-httpd;;;;ERROR/libmicrohttpd;;;;ERROR/util;;;;ERROR/"
 # Run test...
 for n in afl-tests/*
 do
   echo -n "Test $n "
-  taler-mint-httpd -d test-mint-home/ -t 1 -f $n -C > /dev/null || { echo "FAIL!"; exit 1; }
+  $PREFIX taler-mint-httpd -d test-mint-home/ -t 1 -f $n -C > /dev/null || { echo "FAIL!"; }
+#  $PREFIX taler-mint-httpd -d test-mint-home/ -t 1 -f $n -C > /dev/null || { echo "FAIL!"; exit 1; }
   echo "OK"
 done
 exit 0
-- 
cgit v1.2.3