From 417c7c03f4de0c0743fb3dc87d1bf68622896401 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Tue, 31 May 2022 14:18:33 +0200 Subject: [PATCH] BUG/MEDIUM: h3: fix H3_EXCESSIVE_LOAD when receiving H3 frame header only The H3 frame demuxing code is incorrect when receiving a STREAM frame which contains only a new H3 frame header without its payload. In this case, the check on frames bigger than the buffer size is incorrect. This is because the buffer has been freed via qcs_consume()/qc_free_ncbuf() as it was emptied after H3 frame header parsing. This causes the connection to be incorrectly closed with H3_EXCESSIVE_LOAD error. This bug was reproduced with xquic client on the interop and with the command-line invocation : $ ./interop_client -l d -k $SSLKEYLOGFILE -a -p -D /tmp \ -A h3 -U https://:/hello_world.txt Note also that h3_is_frame_valid() invocation has been moved before the new buffer size check. This ensures that first we check the frame validity before returning from the function. It's also better positionned as this is only needed when a new H3 frame header has been parsed. --- src/h3.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/h3.c b/src/h3.c index 24162a2ab..831dc19de 100644 --- a/src/h3.c +++ b/src/h3.c @@ -612,17 +612,20 @@ static int h3_decode_qcs(struct qcs *qcs, int fin, void *ctx) h3s->demux_frame_type = ftype; h3s->demux_frame_len = flen; + + if (!h3_is_frame_valid(h3c, qcs, ftype)) { + qcc_emit_cc_app(qcs->qcc, H3_FRAME_UNEXPECTED); + return 1; + } + qcs_consume(qcs, hlen); + if (!ncb_data(rxbuf, 0)) + break; } flen = h3s->demux_frame_len; ftype = h3s->demux_frame_type; - if (!h3_is_frame_valid(h3c, qcs, ftype)) { - qcc_emit_cc_app(qcs->qcc, H3_FRAME_UNEXPECTED); - return 1; - } - /* Do not demux incomplete frames except H3 DATA which can be * fragmented in multiple HTX blocks. */