hs_pow: Fix nonce cache entry leak

This leak was showing up in address sanitizer runs of test_hs_pow,
but it will also happen during normal operation as seeds are rotated.

Signed-off-by: Micah Elizabeth Scott <beth@torproject.org>

diff --git a/src/feature/hs/hs_pow.c b/src/feature/hs/hs_pow.c
index 77fec689e4aa050dff0428a19eba273aefc5feff..fef809e369060481740a8dfc414399b0c91b47a8 100644 (file)
--- a/src/feature/hs/hs_pow.c
+++ b/src/feature/hs/hs_pow.c
@@ -57,13 +57,18 @@ HT_GENERATE2(nonce_cache_table_ht, nonce_cache_entry_t, node,
              nonce_cache_entry_hash_, nonce_cache_entries_eq_, 0.6,
              tor_reallocarray_, tor_free_);
 
-/** We use this to check if an entry in the replay cache is for a particular
- * seed head, so we know to remove it once the seed is no longer in use. */
+/** This is a callback used to check replay cache entries against a provided
+ * seed head, or NULL to operate on the entire cache. Matching entries return
+ * 1 and their internal cache entry is freed, non-matching entries return 0. */
 static int
-nonce_cache_entry_has_seed(nonce_cache_entry_t *ent, void *data)
+nonce_cache_entry_match_seed_and_free(nonce_cache_entry_t *ent, void *data)
 {
-  /* Returning nonzero makes HT_FOREACH_FN remove the element from the HT */
-  return fast_memeq(ent->bytes.seed_head, data, HS_POW_SEED_HEAD_LEN);
+  if (data == NULL ||
+      fast_memeq(ent->bytes.seed_head, data, HS_POW_SEED_HEAD_LEN)) {
+    tor_free(ent);
+    return 1;
+  }
+  return 0;
 }
 
 /** Helper: Increment a given nonce and set it in the challenge at the right
@@ -298,13 +303,13 @@ hs_pow_verify(const hs_pow_service_state_t *pow_state,
 }
 
 /** Remove entries from the (nonce, seed) replay cache which are for the seed
- * beginning with seed_head. */
+ * beginning with seed_head. If seed_head is NULL, remove all cache entries. */
 void
 hs_pow_remove_seed_from_cache(const uint8_t *seed_head)
 {
   /* If nonce_cache_entry_has_seed returns 1, the entry is removed. */
   HT_FOREACH_FN(nonce_cache_table_ht, &nonce_cache_table,
-                nonce_cache_entry_has_seed, (void*)seed_head);
+                nonce_cache_entry_match_seed_and_free, (void*)seed_head);
 }
 
 /** Free a given PoW service state. */

diff --git a/src/test/test_hs_pow.c b/src/test/test_hs_pow.c
index 877f022a7934086b69dd14b6e84b4ca6298eba52..edda3cd6435b6e14127b7223cddcce0fcd80df98 100644 (file)
--- a/src/test/test_hs_pow.c
+++ b/src/test/test_hs_pow.c
@@ -469,6 +469,7 @@ test_hs_pow_vectors(void *arg)
   trn_cell_introduce1_free(cell);
   trn_cell_introduce_encrypted_free(enc_cell);
   testing_hs_pow_service_free(tsvc);
+  hs_pow_remove_seed_from_cache(NULL);
 }
 
 struct testcase_t hs_pow_tests[] = {

diff --git a/src/test/test_hs_pow_slow.c b/src/test/test_hs_pow_slow.c
index 0c839226469abdc470255e04b832bee7add624ef..13d7111e8854e76e24e95ea4709b66f41d1107be 100644 (file)
--- a/src/test/test_hs_pow_slow.c
+++ b/src/test/test_hs_pow_slow.c
@@ -228,6 +228,7 @@ test_hs_pow_vectors(void *arg)
 
  done:
   testing_disable_prefilled_rng();
+  hs_pow_remove_seed_from_cache(NULL);
 }
 
 struct testcase_t slow_hs_pow_tests[] = {