Opened 12 years ago

Last modified 7 years ago

#446 closed defect (Fixed)

we don't check which hop a cell is from well enough

Reported by: arma Owned by: arma
Priority: Low Milestone:
Component: Core Tor/Tor Version: 0.1.2.14
Severity: Keywords:
Cc: arma, rovv Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

If the streamid of one stream on our circuit collides with the streamid of another
stream (say, because they exit at different hops), this goes bad. There may also be
an attack here where intermediate hops can try to inject cells into streams that
are supposed to be at different hops.

(Somebody should look at this harder before we blindly put the patch in.)

Patch from lodger:

--- relay.c Fri May 25 06:51:40 2007
+++ relay.c Tue Jun 5 07:30:38 2007
@@ -18,5 +18,5 @@

crypt_path_t layer_hint, char *recognized);

static edge_connection_t *relay_lookup_conn(circuit_t *circ, cell_t *cell,

  • int cell_direction);

+ int cell_direction, crypt_path_t *layer_hint);

static int

@@ -163,5 +163,6 @@

if (recognized) {

  • edge_connection_t *conn = relay_lookup_conn(circ, cell, cell_direction);

+ edge_connection_t *conn = relay_lookup_conn(circ, cell, cell_direction,
+ layer_hint);

if (cell_direction == CELL_DIRECTION_OUT) {

++stats_n_relay_cells_delivered;

@@ -373,5 +374,6 @@

*/

static edge_connection_t *

-relay_lookup_conn(circuit_t *circ, cell_t *cell, int cell_direction)
+relay_lookup_conn(circuit_t *circ, cell_t *cell, int cell_direction,
+ crypt_path_t *layer_hint)

{

edge_connection_t *tmpconn;

@@ -391,5 +393,6 @@

tmpconn=tmpconn->next_stream) {

if (rh.stream_id == tmpconn->stream_id &&

  • !tmpconn->_base.marked_for_close) {

+ !tmpconn->_base.marked_for_close &&
+ tmpconn->cpath_layer == layer_hint) {

log_debug(LD_APP,"found conn for stream %d.", rh.stream_id);
return tmpconn;

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (7)

comment:1 Changed 12 years ago by nickm

Looks good; please apply and backport to 0.1.2 and 0.1.1.

comment:2 Changed 12 years ago by arma

Checked in as r10818
Thanks!
(Should also be backported to 0.1.2 at some point)

comment:3 Changed 11 years ago by nickm

Reopened by request from rovv.

comment:4 Changed 11 years ago by rovv

addon:

--- relay.original.c Sun Oct 19 04:31:08 2008
+++ relay.c Sun Oct 19 19:17:56 2008
@@ -1151,6 +1151,12 @@

case RELAY_COMMAND_RENDEZVOUS2:
case RELAY_COMMAND_INTRO_ESTABLISHED:
case RELAY_COMMAND_RENDEZVOUS_ESTABLISHED:

+ if (layer_hint &&
+ layer_hint != TO_ORIGIN_CIRCUIT(circ)->cpath->prev) {
+ log_fn(LOG_PROTOCOL_WARN, LD_APP,
+ "relay cell (rend purpose) from wrong hop; dropping.");
+ return 0;
+ }

rend_process_relay_cell(circ, rh.command, rh.length,

cell->payload+RELAY_HEADER_SIZE);

return 0;

comment:5 Changed 11 years ago by nickm

I've unified this code with the other checks in rend_process_relay_cell; thanks!

comment:6 Changed 11 years ago by nickm

flyspray2trac: bug closed.

comment:7 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.