From 79b59a2dfcb68897ee89d98587d09e55f07e68d7 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 5 Jun 2017 09:35:03 -0400 Subject: [PATCH 1/2] 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 Signed-off-by: David Goulet --- changes/trove-2017-004 | 5 +++++ src/or/connection_edge.c | 21 ++++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 changes/trove-2017-004 diff --git a/changes/trove-2017-004 b/changes/trove-2017-004 new file mode 100644 index 000000000..106d3afcb --- /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 dac0c0101..d9d9e7364 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); From 0c46dc8097a3231cb2e5cb56365a8db8860b4079 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 8 Jun 2017 09:16:33 -0400 Subject: [PATCH 2/2] tweak changes file. --- changes/trove-2017-004 | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/changes/trove-2017-004 b/changes/trove-2017-004 index 106d3afcb..aa901456b 100644 --- a/changes/trove-2017-004 +++ b/changes/trove-2017-004 @@ -1,5 +1,8 @@ - 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. + o Major bugfixes (hidden service, relay, security): + - 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(). Fixes bug 22493, tracked as + TROVE-2017-004 and as CVE-2017-0375; bugfix on tor-0.3.0.1-alpha. + Found by armadev. + +