[tor-commits] [tor/release-0.3.0] TROVE-2017-004: Fix assertion failure in relay_send_end_cell_from_edge_

[nickm at torproject.org]

commit 79b59a2dfcb68897ee89d98587d09e55f07e68d7
Author: David Goulet <dgoulet at torproject.org>
Date:   Mon Jun 5 09:35:03 2017 -0400

    TROVE-2017-004: Fix assertion failure in relay_send_end_cell_from_edge_
    
    This fixes an assertion failure in relay_send_end_cell_from_edge_() when an
    origin circuit and a cpath_layer = NULL were passed.
    
    A service rendezvous circuit could do such a thing when a malformed BEGIN cell
    is received but shouldn't in the first place because the service needs to send
    an END cell on the circuit for which it can not do without a cpath_layer.
    
    Fixes #22493
    
    Reported-by: Roger Dingledine <arma at torproject.org>
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 changes/trove-2017-004   |  5 +++++
 src/or/connection_edge.c | 21 ++++++++++++++-------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/changes/trove-2017-004 b/changes/trove-2017-004
new file mode 100644
index 0000000..106d3af
--- /dev/null
+++ b/changes/trove-2017-004
@@ -0,0 +1,5 @@
+  o Major bugfixes (hidden service, relay):
+    - Fix an assertion failure when an hidden service handles a malformed
+      BEGIN cell. This bug resulted in the service crashing triggered by a
+      tor_assert(). Part of TROVE-2017-004. Fixes bug 22493; bugfix on
+      tor-0.3.0.1-alpha. Found by armadev.
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index dac0c01..d9d9e73 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -3091,14 +3091,21 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   char *address = NULL;
   uint16_t port = 0;
   or_circuit_t *or_circ = NULL;
+  origin_circuit_t *origin_circ = NULL;
+  crypt_path_t *layer_hint = NULL;
   const or_options_t *options = get_options();
   begin_cell_t bcell;
   int rv;
   uint8_t end_reason=0;
 
   assert_circuit_ok(circ);
-  if (!CIRCUIT_IS_ORIGIN(circ))
+  if (!CIRCUIT_IS_ORIGIN(circ)) {
     or_circ = TO_OR_CIRCUIT(circ);
+  } else {
+    tor_assert(circ->purpose == CIRCUIT_PURPOSE_S_REND_JOINED);
+    origin_circ = TO_ORIGIN_CIRCUIT(circ);
+    layer_hint = origin_circ->cpath->prev;
+  }
 
   relay_header_unpack(&rh, cell->payload);
   if (rh.length > RELAY_PAYLOAD_SIZE)
@@ -3123,7 +3130,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
     return -END_CIRC_REASON_TORPROTOCOL;
   } else if (rv == -1) {
     tor_free(bcell.address);
-    relay_send_end_cell_from_edge(rh.stream_id, circ, end_reason, NULL);
+    relay_send_end_cell_from_edge(rh.stream_id, circ, end_reason, layer_hint);
     return 0;
   }
 
@@ -3160,7 +3167,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
     if (!directory_permits_begindir_requests(options) ||
         circ->purpose != CIRCUIT_PURPOSE_OR) {
       relay_send_end_cell_from_edge(rh.stream_id, circ,
-                                    END_STREAM_REASON_NOTDIRECTORY, NULL);
+                                  END_STREAM_REASON_NOTDIRECTORY, layer_hint);
       return 0;
     }
     /* Make sure to get the 'real' address of the previous hop: the
@@ -3177,7 +3184,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   } else {
     log_warn(LD_BUG, "Got an unexpected command %d", (int)rh.command);
     relay_send_end_cell_from_edge(rh.stream_id, circ,
-                                  END_STREAM_REASON_INTERNAL, NULL);
+                                  END_STREAM_REASON_INTERNAL, layer_hint);
     return 0;
   }
 
@@ -3188,7 +3195,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
     if (bcell.flags & BEGIN_FLAG_IPV4_NOT_OK) {
       tor_free(address);
       relay_send_end_cell_from_edge(rh.stream_id, circ,
-                                    END_STREAM_REASON_EXITPOLICY, NULL);
+                                    END_STREAM_REASON_EXITPOLICY, layer_hint);
       return 0;
     }
   }
@@ -3211,7 +3218,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   n_stream->deliver_window = STREAMWINDOW_START;
 
   if (circ->purpose == CIRCUIT_PURPOSE_S_REND_JOINED) {
-    origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ);
+    tor_assert(origin_circ);
     log_info(LD_REND,"begin is for rendezvous. configuring stream.");
     n_stream->base_.address = tor_strdup("(rendezvous)");
     n_stream->base_.state = EXIT_CONN_STATE_CONNECTING;
@@ -3231,7 +3238,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
        * the hidden service. */
       relay_send_end_cell_from_edge(rh.stream_id, circ,
                                     END_STREAM_REASON_DONE,
-                                    origin_circ->cpath->prev);
+                                    layer_hint);
       connection_free(TO_CONN(n_stream));
       tor_free(address);