Fix small easy bugs all around

- Fix log message format string.
- Do extra circuit purpose check.
- wipe memory in a clear function
- Make sure we don't double add intro points in our list
- Make sure we don't double close intro circuits.
- s/tt_u64_op/tt_i64_op/

src/or/hs_cell.c

@@ -169,9 +169,9 @@ parse_introduce2_encrypted(const uint8_t *decrypted_data,
 
   if (trn_cell_introduce_encrypted_getlen_onion_key(enc_cell) !=
       CURVE25519_PUBKEY_LEN) {
-    log_info(LD_REND, "INTRODUCE2 onion key length is invalid. Got %ld but "
+    log_info(LD_REND, "INTRODUCE2 onion key length is invalid. Got %u but "
                       "expected %d on circuit %u for service %s",
-             trn_cell_introduce_encrypted_getlen_onion_key(enc_cell),
+             (unsigned)trn_cell_introduce_encrypted_getlen_onion_key(enc_cell),
              CURVE25519_PUBKEY_LEN, TO_CIRCUIT(circ)->n_circ_id,
              safe_str_client(service->onion_address));
     goto err;

src/or/hs_circuit.c

@@ -898,6 +898,10 @@ hs_circ_handle_intro_established(const hs_service_t *service,
   tor_assert(circ);
   tor_assert(payload);
 
+  if (BUG(TO_CIRCUIT(circ)->purpose != CIRCUIT_PURPOSE_S_ESTABLISH_INTRO)) {
+    goto done;
+  }
+
   /* Try to parse the payload into a cell making sure we do actually have a
    * valid cell. For a legacy node, it's an empty payload so as long as we
    * have the cell, we are good. */

src/or/hs_intropoint.c

@@ -607,5 +607,6 @@ hs_intropoint_clear(hs_intropoint_t *ip)
   SMARTLIST_FOREACH(ip->link_specifiers, hs_desc_link_specifier_t *, ls,
                     hs_desc_link_specifier_free(ls));
   smartlist_free(ip->link_specifiers);
+  memset(ip, 0, sizeof(hs_intropoint_t));
 }
 

src/or/hs_service.c

@@ -433,14 +433,18 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy)
 STATIC void
 service_intro_point_add(digest256map_t *map, hs_service_intro_point_t *ip)
 {
+  hs_service_intro_point_t *old_ip_entry;
+
   tor_assert(map);
   tor_assert(ip);
 
-  digest256map_set(map, ip->auth_key_kp.pubkey.pubkey, ip);
+  old_ip_entry = digest256map_set(map, ip->auth_key_kp.pubkey.pubkey, ip);
+  /* Make sure we didn't just try to double-add an intro point */
+  tor_assert_nonfatal(!old_ip_entry);
 }
 
-/* For a given service, remove the intro point from that service which will
- * look in both descriptors. */
+/* For a given service, remove the intro point from that service's descriptors
+ * (check both current and next descriptor) */
 STATIC void
 service_intro_point_remove(const hs_service_t *service,
                            const hs_service_intro_point_t *ip)
@@ -1623,7 +1627,7 @@ cleanup_intro_points(hs_service_t *service, time_t now)
          * descriptor created and uploaded. There is no difference to an
          * attacker between the timing of a new consensus and intro point
          * rotation (possibly?). */
-        if (ocirc) {
+        if (ocirc && !TO_CIRCUIT(ocirc)->marked_for_close) {
           /* After this, no new cells will be handled on the circuit. */
           circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED);
         }

src/test/test_hs_cell.c

@@ -106,7 +106,7 @@ test_gen_establish_intro_cell_bad(void *arg)
   expect_log_msg_containing("Unable to make signature for "
                             "ESTABLISH_INTRO cell.");
   teardown_capture_of_logs();
-  tt_u64_op(cell_len, OP_EQ, -1);
+  tt_i64_op(cell_len, OP_EQ, -1);
 
  done:
   trn_cell_establish_intro_free(cell);