[tor-commits] [tor/master] relay: Check for NULL arguments in circuitbuild

[nickm at torproject.org]

commit 4f9f56be473d9bc4d24075a82b981425470ff5b4
Author: teor <teor at torproject.org>
Date:   Wed Apr 1 22:25:10 2020 +1000

    relay: Check for NULL arguments in circuitbuild
    
    Part of 33633.
---
 src/feature/relay/circuitbuild_relay.c | 63 ++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/src/feature/relay/circuitbuild_relay.c b/src/feature/relay/circuitbuild_relay.c
index 5c3f0d7d2..49a1146ff 100644
--- a/src/feature/relay/circuitbuild_relay.c
+++ b/src/feature/relay/circuitbuild_relay.c
@@ -42,6 +42,8 @@
 /* Before replying to an extend cell, check the state of the circuit
  * <b>circ</b>, and the configured tor mode.
  *
+ * <b>circ</b> must not be NULL.
+ *
  * If the state and mode are valid, return 0.
  * Otherwise, if they are invalid, log a protocol warning, and return -1.
  */
@@ -53,11 +55,16 @@ circuit_extend_state_valid_helper(const struct circuit_t *circ)
     return -1;
   }
 
+  IF_BUG_ONCE(!circ) {
+    return -1;
+  }
+
   if (circ->n_chan) {
     log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
            "n_chan already set. Bug/attack. Closing.");
     return -1;
   }
+
   if (circ->n_hop) {
     log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
            "conn to next hop already launched. Bug/attack. Closing.");
@@ -81,6 +88,10 @@ circuit_extend_state_valid_helper(const struct circuit_t *circ)
 STATIC int
 circuit_extend_add_ed25519_helper(struct extend_cell_t *ec)
 {
+  IF_BUG_ONCE(!ec) {
+    return -1;
+  }
+
   /* Check if they asked us for 0000..0000. We support using
    * an empty fingerprint for the first hop (e.g. for a bridge relay),
    * but we don't want to let clients send us extend cells for empty
@@ -120,6 +131,14 @@ STATIC int
 circuit_extend_lspec_valid_helper(const struct extend_cell_t *ec,
                                   const struct circuit_t *circ)
 {
+  IF_BUG_ONCE(!ec) {
+    return -1;
+  }
+
+  IF_BUG_ONCE(!circ) {
+    return -1;
+  }
+
   if (!ec->orport_ipv4.port || tor_addr_is_null(&ec->orport_ipv4.addr)) {
     log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
            "Client asked me to extend to zero destination port or addr.");
@@ -133,8 +152,16 @@ circuit_extend_lspec_valid_helper(const struct extend_cell_t *ec,
     return -1;
   }
 
+  IF_BUG_ONCE(circ->magic != OR_CIRCUIT_MAGIC) {
+    return -1;
+  }
+
   const channel_t *p_chan = CONST_TO_OR_CIRCUIT(circ)->p_chan;
 
+  IF_BUG_ONCE(!p_chan) {
+    return -1;
+  }
+
   /* Next, check if we're being asked to connect to the hop that the
    * extend cell came from. There isn't any reason for that, and it can
    * assist circular-path attacks. */
@@ -166,6 +193,18 @@ circuit_open_connection_for_extend(const struct extend_cell_t *ec,
                                    struct circuit_t *circ,
                                    int should_launch)
 {
+  /* We have to check circ first, so we can close it on all other failures */
+  IF_BUG_ONCE(!circ) {
+    /* We can't mark a NULL circuit for close. */
+    return;
+  }
+
+  /* Now we know that circ is not NULL */
+  IF_BUG_ONCE(!ec) {
+    circuit_mark_for_close(circ, END_CIRC_REASON_CONNECTFAILED);
+    return;
+  }
+
   circ->n_hop = extend_info_new(NULL /*nickname*/,
                                 (const char*)ec->node_id,
                                 &ec->ed_pubkey,
@@ -211,6 +250,14 @@ circuit_extend(struct cell_t *cell, struct circuit_t *circ)
   const char *msg = NULL;
   int should_launch = 0;
 
+  IF_BUG_ONCE(!cell) {
+    return -1;
+  }
+
+  IF_BUG_ONCE(!circ) {
+    return -1;
+  }
+
   if (circuit_extend_state_valid_helper(circ) < 0)
     return -1;
 
@@ -289,6 +336,22 @@ onionskin_answer(struct or_circuit_t *circ,
 {
   cell_t cell;
 
+  IF_BUG_ONCE(!circ) {
+    return -1;
+  }
+
+  IF_BUG_ONCE(!created_cell) {
+    return -1;
+  }
+
+  IF_BUG_ONCE(!keys) {
+    return -1;
+  }
+
+  IF_BUG_ONCE(!rend_circ_nonce) {
+    return -1;
+  }
+
   tor_assert(keys_len == CPATH_KEY_MATERIAL_LEN);
 
   if (created_cell_format(&cell, created_cell) < 0) {