Browse Source

Fix ancient code that only checked circ_id, not circ_id and chan

This code mis-handled the case where a circuit got the same circuit
ID in both directions.  I found three instances of it in the
codebase, by grepping for [pn]_circ_id.

Because of the issue in command_process_relay_cell(), this would
have made roughly one circuit in a million completely nonfunctional.

Fixes bug 12195.
Nick Mathewson 10 years ago
parent
commit
84ed086d48
2 changed files with 10 additions and 1 deletions
  1. 7 0
      changes/bug12195
  2. 3 1
      src/or/command.c

+ 7 - 0
changes/bug12195

@@ -0,0 +1,7 @@
+  o Major bugfixes:
+    - When a circuit accidentally has the same circuit ID for its
+      forward and reverse direction, correctly detect the direction of
+      cells using that circuit. Previously, this would have made
+      roughly one circuit in a million non-functional. Fixes bug
+      12195; this is a bugfix on every version of Tor.
+

+ 3 - 1
src/or/command.c

@@ -349,7 +349,7 @@ command_process_created_cell(cell_t *cell, channel_t *chan)
     return;
   }
 
-  if (circ->n_circ_id != cell->circ_id) {
+  if (circ->n_circ_id != cell->circ_id || circ->n_chan != chan) {
     log_fn(LOG_PROTOCOL_WARN,LD_PROTOCOL,
            "got created cell from Tor client? Closing.");
     circuit_mark_for_close(circ, END_CIRC_REASON_TORPROTOCOL);
@@ -434,6 +434,7 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
   }
 
   if (!CIRCUIT_IS_ORIGIN(circ) &&
+      chan == TO_OR_CIRCUIT(circ)->p_chan &&
       cell->circ_id == TO_OR_CIRCUIT(circ)->p_circ_id)
     direction = CELL_DIRECTION_OUT;
   else
@@ -501,6 +502,7 @@ command_process_destroy_cell(cell_t *cell, channel_t *chan)
   reason = (uint8_t)cell->payload[0];
 
   if (!CIRCUIT_IS_ORIGIN(circ) &&
+      chan == TO_OR_CIRCUIT(circ)->p_chan &&
       cell->circ_id == TO_OR_CIRCUIT(circ)->p_circ_id) {
     /* the destroy came from behind */
     circuit_set_p_circid_chan(TO_OR_CIRCUIT(circ), 0, NULL);