CVE-2017-0375: Reachable Assertion (CWE-617)
value: The hidden-service feature in Tor before 0.3.0.8 allows a denial of service (assertion failure and daemon exit) in the relay_send_end_cell_from_edge_ function via a malformed BEGIN cell.
CWE description: The product contains an assert() or similar statement that can be triggered by an attacker, which leads to an application exit or other behavior that is more severe than necessary.
CVSS v3 base score: 7.5
Published: 2017-06-09T17:29Z
Repository: https://github.com/torproject/tor
Language: Other
Files changed: changes/trove-2017-004, src/or/connection_edge.c
Commit date: 2017-06-08 09:14:10 -0400
Commit message: 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@torproject.org>
Signed-off-by: David Goulet <dgoulet@torproject.org>

Diff: diff --git a/changes/trove-2017-004 b/changes/trove-2017-004
new file mode 100644
index 0000000000..106d3afcb5
--- /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 dac0c01012..d9d9e73643 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -3089,18 +3089,25 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   edge_connection_t *n_stream;
   relay_header_t rh;
   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)
     return -END_CIRC_REASON_TORPROTOCOL;
 
@@ -3121,11 +3128,11 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   rv = begin_cell_parse(cell, &bcell, &end_reason);
   if (rv < -1) {
     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;
   }
 
   if (! bcell.is_begindir) {
     /* Steal reference */
@@ -3158,11 +3165,11 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
     }
   } else if (rh.command == RELAY_COMMAND_BEGIN_DIR) {
     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
      * caller might want to know whether the remote IP address has changed,
      * and we might already have corrected base_.addr[ess] for the relay's
@@ -3175,22 +3182,22 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
                * isn't "really" a connection here.  But we
                * need to set it to something nonzero. */
   } 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;
   }
 
   if (! options->IPv6Exit) {
     /* I don't care if you prefer IPv6; I can't give you any. */
     bcell.flags &= ~BEGIN_FLAG_IPV6_PREFERRED;
     /* If you don't want IPv4, I can't help. */
     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;
     }
   }
 
   log_debug(LD_EXIT,"Creating new exit connection.");
@@ -3209,11 +3216,11 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
   /* leave n_stream->s at -1, because it's not yet valid */
   n_stream->package_window = STREAMWINDOW_START;
   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;
     n_stream->rend_data = rend_data_dup(origin_circ->rend_data);
     tor_assert(connection_edge_is_rendezvous_stream(n_stream));
@@ -3229,11 +3236,11 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
        * return DONE and kill the circuit. That way, a user (malicious or
        * not) needs one circuit per bad port unless it matches the policy of
        * 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);
 
       /* Drop the circuit here since it might be someone deliberately
        * scanning the hidden service ports. Note that this mitigates port

Vulnerable code:   if (!CIRCUIT_IS_ORIGIN(circ))
    relay_send_end_cell_from_edge(rh.stream_id, circ, end_reason, NULL);
                                    END_STREAM_REASON_NOTDIRECTORY, NULL);
                                  END_STREAM_REASON_INTERNAL, NULL);
                                    END_STREAM_REASON_EXITPOLICY, NULL);
    origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ);
                                    origin_circ->cpath->prev);
Fixed code:   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.
  origin_circuit_t *origin_circ = NULL;
  crypt_path_t *layer_hint = NULL;
  if (!CIRCUIT_IS_ORIGIN(circ)) {
  } else {
    tor_assert(circ->purpose == CIRCUIT_PURPOSE_S_REND_JOINED);
    origin_circ = TO_ORIGIN_CIRCUIT(circ);
    layer_hint = origin_circ->cpath->prev;
  }
    relay_send_end_cell_from_edge(rh.stream_id, circ, end_reason, layer_hint);
                                  END_STREAM_REASON_NOTDIRECTORY, layer_hint);
                                  END_STREAM_REASON_INTERNAL, layer_hint);
                                    END_STREAM_REASON_EXITPOLICY, layer_hint);
    tor_assert(origin_circ);
                                    layer_hint);