From 88d5dd1a6f4c48db667c3cc7de8dd5110af5409d Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Tue, 31 May 2022 11:44:52 +0200 Subject: [PATCH] BUG/MINOR: h3: fix frame demuxing The H3 demuxing code was not fully correct. After parsing the H3 frame header, the check between frame length and buffer data is wrong as we compare a copy of the buffer made before the H3 header removal. Fix this by improving the H3 demuxing code API. h3_decode_frm_header() now uses a ncbuf instance, this prevents an unnecessary cast ncbuf/buffer in h3_decode_qcs() which resolves this error. This bug was not triggered at this moment. Its impact should be really limited. --- src/h3.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/h3.c b/src/h3.c index 447a8587e..24162a2ab 100644 --- a/src/h3.c +++ b/src/h3.c @@ -254,19 +254,23 @@ static int h3_parse_uni_stream_no_h3(struct qcs *qcs, struct ncbuf *rxbuf) return 0; } -/* Decode a h3 frame header made of two QUIC varints from buffer. - * Returns the number of bytes consumed if there was enough data in , 0 if not. - * Note that this function update buffer to reflect the number of bytes consumed - * to decode the h3 frame header. +/* Decode a H3 frame header from buffer. The frame type is stored in + * and length in . + * + * Returns the size of the H3 frame header. Note that the input buffer is not + * consumed. */ static inline size_t h3_decode_frm_header(uint64_t *ftype, uint64_t *flen, - struct buffer *b) + struct ncbuf *rxbuf) { size_t hlen; + struct buffer b = h3_b_dup(rxbuf); hlen = 0; - if (!b_quic_dec_int(ftype, b, &hlen) || !b_quic_dec_int(flen, b, &hlen)) + if (!b_quic_dec_int(ftype, &b, &hlen) || + !b_quic_dec_int(flen, &b, &hlen)) { return 0; + } return hlen; } @@ -595,13 +599,11 @@ static int h3_decode_qcs(struct qcs *qcs, int fin, void *ctx) while (ncb_data(rxbuf, 0) && !(qcs->flags & QC_SF_DEM_FULL)) { uint64_t ftype, flen; - struct buffer b; char last_stream_frame = 0; /* Work on a copy of */ - b = h3_b_dup(rxbuf); if (!h3s->demux_frame_len) { - size_t hlen = h3_decode_frm_header(&ftype, &flen, &b); + size_t hlen = h3_decode_frm_header(&ftype, &flen, rxbuf); if (!hlen) break; @@ -624,7 +626,7 @@ static int h3_decode_qcs(struct qcs *qcs, int fin, void *ctx) /* Do not demux incomplete frames except H3 DATA which can be * fragmented in multiple HTX blocks. */ - if (flen > b_data(&b) && ftype != H3_FT_DATA) { + if (flen > ncb_data(rxbuf, 0) && ftype != H3_FT_DATA) { /* Reject frames bigger than bufsize. * * TODO HEADERS should in complement be limited with H3