tor

commit 9d238778c7c034d7ed68aa9a642de0dd1d7e79cc
parent d433fd753d86992d071c015ddca65fd2563a10a1
Author: David Goulet <dgoulet@torproject.org>
Date:   Wed, 12 Nov 2025 08:52:36 -0500

conflux: Check if circuit is closed after cell dequeue from OOO q

Unfortunately, a circuit can get close between two conflux cell processing from
the OOO q. Because no error is returned in that case, it leads to a possible
NULL deref.

Fixes #41162

Signed-off-by: David Goulet <dgoulet@torproject.org>

Diffstat:
A
changes/ticket41162
 | 
5
+++++
M
src/core/or/conflux.c
 | 
31
++++++++++++++++++++++++++++++-
M
src/core/or/conflux.h
 | 
2
+-
M
src/core/or/relay.c
 | 
2
+-

4 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/changes/ticket41162 b/changes/ticket41162
@@ -0,0 +1,5 @@
+  o Major bugfixes (conflux, exit):
+    - When dequeuing out-of-order conflux cells, the circuit could be close in
+      between two dequeue which could lead to a mishandling of a NULL pointer.
+      Fixes bug 41162; bugfix on 0.4.8.4.
+
diff --git a/src/core/or/conflux.c b/src/core/or/conflux.c
@@ -919,9 +919,38 @@ conflux_process_cell(conflux_t *cfx, circuit_t *in_circ,
  * or has a hole.
  */
 conflux_cell_t *
-conflux_dequeue_cell(conflux_t *cfx)
+conflux_dequeue_cell(circuit_t *circ)
 {
   conflux_cell_t *top = NULL;
+
+  /* Related to #41162. This is really a consequence of the C-tor maze.
+   * The function above can close a circuit without returning an error
+   * due to several return code ignored. Auditting all of the cell code
+   * path and fixing them to not ignore errors could bring many more
+   * issues as this behavior has been in tor forever. So do the bandaid
+   * fix of bailing if the circuit is closed. */
+  if (circ->marked_for_close) {
+    static ratelim_t rlim = RATELIM_INIT(60 * 60);
+    log_fn_ratelim(&rlim, (circ->conflux == NULL) ? LOG_WARN : LOG_NOTICE,
+                   LD_CIRC,
+                   "Circuit was closed at %s:%u when dequeuing from OOO",
+                   circ->marked_for_close_file, circ->marked_for_close);
+    return NULL;
+  }
+  conflux_t *cfx = circ->conflux;
+  if (cfx == NULL) {
+    static ratelim_t rlim = RATELIM_INIT(60 * 60);
+    log_fn_ratelim(&rlim, LOG_WARN, LD_CIRC,
+                   "Bug: Non marked for close circuit with NULL conflux");
+    return NULL;
+  }
+  if (cfx->ooo_q == NULL) {
+    static ratelim_t rlim = RATELIM_INIT(60 * 60);
+    log_fn_ratelim(&rlim, LOG_WARN, LD_CIRC,
+                   "Bug: Non marked for close circuit with NULL OOO queue");
+    return NULL;
+  }
+
   if (smartlist_len(cfx->ooo_q) == 0)
     return NULL;
 
diff --git a/src/core/or/conflux.h b/src/core/or/conflux.h
@@ -60,7 +60,7 @@ bool conflux_should_multiplex(int relay_command);
 bool conflux_process_cell(conflux_t *cfx, circuit_t *in_circ,
                           crypt_path_t *layer_hint,
                           cell_t *cell);
-conflux_cell_t *conflux_dequeue_cell(conflux_t *cfx);
+conflux_cell_t *conflux_dequeue_cell(circuit_t *circ);
 void conflux_note_cell_sent(conflux_t *cfx, circuit_t *circ,
                             uint8_t relay_command);
 
diff --git a/src/core/or/relay.c b/src/core/or/relay.c
@@ -2173,7 +2173,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       }
 
       /* Now, check queue for more */
-      while ((c_cell = conflux_dequeue_cell(circ->conflux))) {
+      while ((c_cell = conflux_dequeue_cell(circ))) {
         relay_header_unpack(&rh, c_cell->cell.payload);
         conn = relay_lookup_conn(circ, &c_cell->cell, CELL_DIRECTION_OUT,
                                  layer_hint);