1berry/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
Amaury Denoyelle 2025-05-09 10:54:40 +02:00
parent 3f9194bfc9
commit 14e4f2b811
1 changed files with 6 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/mux_quic.c
View File

@ -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)) {
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;