BUG/MEDIUM: mux_quic: prevent uaf on fctl frame after send error

When performing emission in QUIC MUX, flow control frames are emitted
first via qcc_emit_fctl(), with a single frame per packet. Emission is
interrupted if an error is returned by the transport layer via
qcc_send_frames().

For each successful MAX_STREAM_DATA frame emission, <msd_frm> pointer of
QCS must be reset. This is necessary as the frame ownership moved to the
transport layer.

Previously, in case of qcc_send_frames() error, input frame was
reinserted in the MUX send list and QCS ownership of the frame would
remain. This is incorrect though in case of a transient error (for
example EAGAIN), as transport layer now owns the frame as it is
responsible to retry emission later. When the frame is finally emitted
and freed by the transport layer, an UAF may occur when QCS flow control
is updated.

This bug occurs with connection owned sockets only. This is because
transient socket errors are simply ignored when using the listener
socket.

To fix this, rewrite qcc_emit_fctl() error path. Cleanup of <msd_frm> is
now performed systematically if frame input list is empty. The function
is then still interrupted if qcc_send_frames() returned an error.

This must be backported up to 3.2, after a period of observation.

diff --git a/src/mux_quic.c b/src/mux_quic.c
index de443a5682b833cb0a7038fde7e28a47a97487a0..3bfac5d955494f8484f932c00e06daa55fff2ae7 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -3014,6 +3014,7 @@ static int qcc_emit_fctl(struct qcc *qcc)
        struct eb64_node *node;
        struct qcs *qcs;
        int64_t id;
+       int ret;
 
        TRACE_ENTER(QMUX_EV_QCC_END, qcc->conn);
 
@@ -3026,25 +3027,26 @@ static int qcc_emit_fctl(struct qcc *qcc)
                LIST_DELETE(&frm->list);
                LIST_APPEND(&single_frm, &frm->list);
 
-               if (qcc_send_frames(qcc, &single_frm, 0)) {
-                       if (!LIST_ISEMPTY(&single_frm)) {
-                               /* replace frame that cannot be sent in qcc list. */
-                               LIST_DELETE(&frm->list);
-                               LIST_INSERT(&qcc->lfctl.frms, &frm->list);
-                       }
-                       goto err;
+               ret = qcc_send_frames(qcc, &single_frm, 0);
+               if (!LIST_ISEMPTY(&single_frm)) {
+                       /* replace frame that cannot be sent in qcc list. */
+                       LIST_DELETE(&frm->list);
+                       LIST_INSERT(&qcc->lfctl.frms, &frm->list);
                }
-
-               BUG_ON_HOT(!LIST_ISEMPTY(&single_frm));
-
-               /* If frame is MAX_STREAM_DATA, reset corresponding QCS msd_frm pointer. */
-               if (id >= 0) {
+               else if (id >= 0) {
+                       /* Frame now handled by transport layer, either on send
+                        * success or after transient error. For a stream level
+                        * frame, reset QCS <msd_frm> pointer.
+                        */
                        node = eb64_lookup(&qcc->streams_by_id, id);
                        if (node) {
                                qcs = eb64_entry(node, struct qcs, by_id);
                                qcs->tx.msd_frm = NULL;
                        }
                }
+
+               if (ret)
+                       goto err;
        }
 
        TRACE_LEAVE(QMUX_EV_QCC_SEND, qcc->conn);