From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Fri, 9 May 2025 08:54:40 +0000 (+0200)
Subject: BUG/MEDIUM: mux-quic: fix crash on invalid fctl frame dereference
X-Git-Tag: v3.2-dev16~52
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=14e4f2b811ffb215c8b237d4c70228b0a0ca1576;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: mux-quic: fix crash on invalid fctl frame dereference

Emission of flow-control frames have been recently modified. Now, each
frame is sent one by one, via a single entry list. If a failure occurs,
emission is interrupted and frame is reinserted into the original
<qcc.lfctl.frms> list.

This code is incorrect as it only checks if qcc_send_frames() returns an
error code to perform the reinsert operation. However, an error here
does not always mean that the frame was not properly emitted by lower
quic-conn layer. As such, an extra test LIST_ISEMPTY() must be performed
prior to reinsert the frame.

This bug would cause a heap overflow. Indeed, the reinsert frame would
be a random value. A crash would occur as soon as it would be
dereferenced via <qcc.lfctl.frms> list.

This was reproduced by issuing a POST with a big file and interrupt it
after just a few seconds. This results in a crash in about a third of
the tests. Here is an example command using ngtcp2 :

 $ ngtcp2-client -q --no-quic-dump --no-http-dump \
   -m POST -d ~/infra/html/1g 127.0.0.1 20443 "http://127.0.0.1:20443/post"

Heap overflow was detected via a BUG_ON() statement from qc_frm_free()
via qcc_release() caller :

  FATAL: bug condition "!((&((*frm)->reflist))->n == (&((*frm)->reflist)))" matched at src/quic_frame.c:1270

This does not need to be backported.
---

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 68b913970..98438a7a1 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -2616,9 +2616,11 @@ static int qcc_emit_fctl(struct qcc *qcc)
 		LIST_APPEND(&single_frm, &frm->list);
 
 		if (qcc_send_frames(qcc, &single_frm, 0)) {
-			/* replace frame that cannot be sent in qcc list. */
-			LIST_DELETE(&frm->list);
-			LIST_INSERT(&qcc->lfctl.frms, &frm->list);
+			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;
 		}
 
@@ -2626,7 +2628,7 @@ static int qcc_emit_fctl(struct qcc *qcc)
 
 		/* If frame is MAX_STREAM_DATA, reset corresponding QCS msd_frm pointer. */
 		if (id >= 0) {
-			node = eb64_lookup(&qcc->streams_by_id, frm->max_stream_data.id);
+			node = eb64_lookup(&qcc->streams_by_id, id);
 			if (node) {
 				qcs = eb64_entry(node, struct qcs, by_id);
 				qcs->tx.msd_frm = NULL;