[taler-exchange] branch master updated: address fixme, fix use-after-free in testing traits

[gnunet]

This is an automated email from the git hooks/post-receive script.

dold pushed a commit to branch master
in repository exchange.

The following commit(s) were added to refs/heads/master by this push:
     new 33877b2c address fixme, fix use-after-free in testing traits
33877b2c is described below

commit 33877b2c55be2d9a4deaf56c709a7943dd00b62c
Author: Florian Dold <address@hidden>
AuthorDate: Fri Jan 17 20:50:09 2020 +0100

    address fixme, fix use-after-free in testing traits
---
 src/exchange/taler-exchange-httpd_keystate.c |  7 +++---
 src/include/taler_exchange_service.h         | 22 +++++++++++++++++
 src/lib/exchange_api_handle.c                | 37 ++++++++++++++++++++++++++++
 src/lib/testing_api_cmd_refresh.c            |  8 ++++++
 src/lib/testing_api_cmd_withdraw.c           | 24 +++++++++++++-----
 5 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/src/exchange/taler-exchange-httpd_keystate.c 
b/src/exchange/taler-exchange-httpd_keystate.c
index 54f4c308..46c08e51 100644
--- a/src/exchange/taler-exchange-httpd_keystate.c
+++ b/src/exchange/taler-exchange-httpd_keystate.c
@@ -1261,12 +1261,11 @@ setup_general_response_headers (const struct 
TEH_KS_StateHandle *key_state,
     m = GNUNET_TIME_relative_to_absolute (TEH_max_keys_caching);
     m = GNUNET_TIME_absolute_min (m,
                                   key_state->next_reload);
-    // FIXME: setting 'm' to FOREVER here exposes
-    // a crash-bug in lib/ where we access /keys
-    // data after it was already free'd!
-    // m = GNUNET_TIME_UNIT_FOREVER_ABS;
     get_date_string (m,
                      dat);
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+                "settings /keys 'Expires' header to '%s'\n",
+                dat);
     GNUNET_break (MHD_YES ==
                   MHD_add_response_header (response,
                                            MHD_HTTP_HEADER_EXPIRES,
diff --git a/src/include/taler_exchange_service.h 
b/src/include/taler_exchange_service.h
index 972b94aa..de1daf1e 100644
--- a/src/include/taler_exchange_service.h
+++ b/src/include/taler_exchange_service.h
@@ -530,6 +530,28 @@ TALER_EXCHANGE_get_denomination_key (const struct 
TALER_EXCHANGE_Keys *keys,
                                      TALER_DenominationPublicKey *pk);
 
 
+/**
+ * Create a copy of a denomination public key.
+ *
+ * @param key key to copy
+ * @returns a copy, must be freed with #TALER_EXCHANGE_destroy_denomination_key
+ */
+struct TALER_EXCHANGE_DenomPublicKey *
+TALER_EXCHANGE_copy_denomination_key (const struct
+                                      TALER_EXCHANGE_DenomPublicKey *key);
+
+
+/**
+ * Destroy a denomination public key.
+ * Should only be called with keys created by 
#TALER_EXCHANGE_copy_denomination_key.
+ *
+ * @param key key to destroy.
+ */
+void
+TALER_EXCHANGE_destroy_denomination_key (struct
+                                         TALER_EXCHANGE_DenomPublicKey *key);
+
+
 /**
  * Obtain the denomination key details from the exchange.
  *
diff --git a/src/lib/exchange_api_handle.c b/src/lib/exchange_api_handle.c
index c3a0598f..d7f56bc8 100644
--- a/src/lib/exchange_api_handle.c
+++ b/src/lib/exchange_api_handle.c
@@ -2127,6 +2127,43 @@ TALER_EXCHANGE_get_denomination_key (const struct 
TALER_EXCHANGE_Keys *keys,
 }
 
 
+/**
+ * Create a copy of a denomination public key.
+ *
+ * @param key key to copy
+ * @returns a copy, must be freed with #TALER_EXCHANGE_destroy_denomination_key
+ */
+struct TALER_EXCHANGE_DenomPublicKey *
+TALER_EXCHANGE_copy_denomination_key (const struct
+                                      TALER_EXCHANGE_DenomPublicKey *key)
+{
+  struct TALER_EXCHANGE_DenomPublicKey *copy;
+
+  copy = GNUNET_new (struct TALER_EXCHANGE_DenomPublicKey);
+  *copy = *key;
+  copy->key.rsa_public_key = GNUNET_CRYPTO_rsa_public_key_dup (
+    key->key.rsa_public_key);
+
+  return copy;
+}
+
+
+/**
+ * Destroy a denomination public key.
+ * Should only be called with keys created by 
#TALER_EXCHANGE_copy_denomination_key.
+ *
+ * @param key key to destroy.
+ */
+void
+TALER_EXCHANGE_destroy_denomination_key (struct
+                                         TALER_EXCHANGE_DenomPublicKey *key)
+{
+  GNUNET_CRYPTO_rsa_public_key_free (key->key.rsa_public_key);;
+  key->key.rsa_public_key = NULL;
+  GNUNET_free (key);
+}
+
+
 /**
  * Obtain the denomination key details from the exchange.
  *
diff --git a/src/lib/testing_api_cmd_refresh.c 
b/src/lib/testing_api_cmd_refresh.c
index 2e921cef..beaec168 100644
--- a/src/lib/testing_api_cmd_refresh.c
+++ b/src/lib/testing_api_cmd_refresh.c
@@ -988,6 +988,9 @@ refresh_melt_run (void *cls,
                                        &melt_amount,
                                        &fresh_pk->fee_withdraw));
       rms->fresh_pks[i] = *fresh_pk;
+      /* Make a deep copy of the RSA key */
+      rms->fresh_pks[i].key.rsa_public_key = GNUNET_CRYPTO_rsa_public_key_dup (
+        fresh_pk->key.rsa_public_key);
     }
     rms->refresh_data = TALER_EXCHANGE_refresh_prepare
                           (rms->melt_priv, &melt_amount, melt_sig,
@@ -1041,6 +1044,11 @@ refresh_melt_cleanup (void *cls,
     GNUNET_SCHEDULER_cancel (rms->retry_task);
     rms->retry_task = NULL;
   }
+  if (NULL != rms->fresh_pks)
+  {
+    for (unsigned int i = 0; i < rms->num_fresh_coins; i++)
+      GNUNET_CRYPTO_rsa_public_key_free (rms->fresh_pks[i].key.rsa_public_key);
+  }
   GNUNET_free_non_null (rms->fresh_pks);
   rms->fresh_pks = NULL;
   GNUNET_free_non_null (rms->refresh_data);
diff --git a/src/lib/testing_api_cmd_withdraw.c 
b/src/lib/testing_api_cmd_withdraw.c
index b01209b5..57b440fa 100644
--- a/src/lib/testing_api_cmd_withdraw.c
+++ b/src/lib/testing_api_cmd_withdraw.c
@@ -55,7 +55,7 @@ struct WithdrawState
    * use.  Otherwise, this will be set (by the interpreter) to the
    * denomination PK matching @e amount.
    */
-  const struct TALER_EXCHANGE_DenomPublicKey *pk;
+  struct TALER_EXCHANGE_DenomPublicKey *pk;
 
   /**
    * Exchange base URL.  Only used as offered trait.
@@ -246,6 +246,7 @@ withdraw_run (void *cls,
   struct WithdrawState *ws = cls;
   const struct TALER_ReservePrivateKeyP *rp;
   const struct TALER_TESTING_Command *create_reserve;
+  const struct TALER_EXCHANGE_DenomPublicKey *dpk;
 
   (void) cmd;
   create_reserve = TALER_TESTING_interpreter_lookup_command
@@ -268,16 +269,21 @@ withdraw_run (void *cls,
   TALER_planchet_setup_random (&ws->ps);
   ws->is = is;
 
-  ws->pk = TALER_TESTING_find_pk
-             (TALER_EXCHANGE_get_keys (is->exchange),
-             &ws->amount);
-  if (NULL == ws->pk)
+  dpk = TALER_TESTING_find_pk (TALER_EXCHANGE_get_keys (is->exchange),
+                               &ws->amount);
+  if (NULL == dpk)
   {
     GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
                 "Failed to determine denomination key at %s\n",
                 cmd->label);
     GNUNET_assert (0);
   }
+  else
+  {
+    /* We copy the denomination key, as re-querying /keys
+     * would free the old one. */
+    ws->pk = TALER_EXCHANGE_copy_denomination_key (dpk);
+  }
 
   ws->wsh = TALER_EXCHANGE_reserve_withdraw (is->exchange,
                                              ws->pk,
@@ -325,6 +331,12 @@ withdraw_cleanup (void *cls,
     GNUNET_CRYPTO_rsa_signature_free (ws->sig.rsa_signature);
     ws->sig.rsa_signature = NULL;
   }
+  if (NULL != ws->pk)
+  {
+    TALER_EXCHANGE_destroy_denomination_key (ws->pk);
+    ws->pk = NULL;
+  }
+
   GNUNET_free_non_null (ws->exchange_url);
   GNUNET_free (ws);
 }
@@ -493,7 +505,7 @@ TALER_TESTING_cmd_withdraw_denomination
   }
   ws = GNUNET_new (struct WithdrawState);
   ws->reserve_reference = reserve_reference;
-  ws->pk = dk;
+  ws->pk = TALER_EXCHANGE_copy_denomination_key (dk);
   ws->expected_response_code = expected_response_code;
   {
     struct TALER_TESTING_Command cmd = {

-- 
To stop receiving notification emails like this one, please contact
address@hidden.