Fix UAF in fd_quic tile

Fixes a assertion failure when closing a conn with streams that got overrun. Adds more logging to assertion failures and hardens the fd_tpu_reasm_cancel function.
firedancer-io · Sep 18, 2024 · 9f75ce9 · 9f75ce9
1 parent 542b0bc
commit 9f75ce9
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 8 deletions.
diff --git a/src/app/fdctl/run/tiles/fd_quic.c b/src/app/fdctl/run/tiles/fd_quic.c
@@ -428,11 +428,6 @@ quic_stream_notify( fd_quic_stream_t * stream,
   fd_frag_meta_t *      mcache = mux->mcache;
   void *                base   = ctx->verify_out_mem;
 
-  if( FD_UNLIKELY( type!=FD_QUIC_NOTIFY_END ) ) {
-    fd_tpu_reasm_cancel( reasm, slot );
-    return;  /* not a successful stream close */
-  }
-
   /* Check if reassembly slot is still valid */
 
   ulong conn_id   = stream->conn->local_conn_id;
@@ -444,6 +439,13 @@ quic_stream_notify( fd_quic_stream_t * stream,
     return;  /* clobbered */
   }
 
+  /* Abort reassembly slot if QUIC stream closes non-gracefully */
+
+  if( FD_UNLIKELY( type!=FD_QUIC_NOTIFY_END ) ) {
+    fd_tpu_reasm_cancel( reasm, slot );
+    return;  /* not a successful stream close */
+  }
+
   /* Publish message */
 
   ulong  seq   = *mux->seq;

diff --git a/src/disco/quic/fd_tpu_reasm.c b/src/disco/quic/fd_tpu_reasm.c
@@ -246,7 +246,10 @@ fd_tpu_reasm_publish( fd_tpu_reasm_t *      reasm,
 void
 fd_tpu_reasm_cancel( fd_tpu_reasm_t *      reasm,
                      fd_tpu_reasm_slot_t * slot ) {
+  if( FD_UNLIKELY( slot->state == FD_TPU_REASM_STATE_FREE ) ) return;
   slotq_remove( reasm, slot );
-  slot->state = FD_TPU_REASM_STATE_FREE;
+  slot->state     = FD_TPU_REASM_STATE_FREE;
+  slot->conn_id   = 0UL;
+  slot->stream_id = 0UL;
   slotq_push_tail( reasm, slot );
 }
diff --git a/src/disco/quic/fd_tpu_reasm_private.h b/src/disco/quic/fd_tpu_reasm_private.h
@@ -139,20 +139,30 @@ slotq_remove( fd_tpu_reasm_t *      reasm,
   fd_tpu_reasm_slot_t * next = fd_tpu_reasm_slots_laddr( reasm ) + next_idx;
 
   if( slot_idx==reasm->head ) {
-    assert( next_idx < reasm->slot_cnt );
+    if( FD_UNLIKELY( next_idx >= reasm->slot_cnt ) ) {
+      FD_LOG_ERR(( "OOB next_idx (next_idx=%u, slot_cnt=%u)", next_idx, reasm->slot_cnt ));
+    }
     reasm->head    = next_idx;
     next->prev_idx = UINT_MAX;
     return;
   }
   if( slot_idx==reasm->tail ) {
-    assert( prev_idx < reasm->slot_cnt );
+    if( FD_UNLIKELY( prev_idx >= reasm->slot_cnt ) ) {
+      FD_LOG_ERR(( "OOB prev_idx (prev_idx=%u, slot_cnt=%u)", prev_idx, reasm->slot_cnt ));
+    }
     reasm->tail    = prev_idx;
     prev->next_idx = UINT_MAX;
     return;
   }
 
   assert( prev_idx < reasm->slot_cnt );
   assert( next_idx < reasm->slot_cnt );
+  if( FD_UNLIKELY( prev_idx >= reasm->slot_cnt ) ) {
+    FD_LOG_ERR(( "OOB prev_idx (prev_idx=%u, slot_cnt=%u)", prev_idx, reasm->slot_cnt ));
+  }
+  if( FD_UNLIKELY( next_idx >= reasm->slot_cnt ) ) {
+    FD_LOG_ERR(( "OOB next_idx (next_idx=%u, slot_cnt=%u)", next_idx, reasm->slot_cnt ));
+  }
   prev->next_idx = next_idx;
   next->prev_idx = prev_idx;
 }