Skip to content

Commit ace7ab7

Browse files
committed
BUG/MEDIUM: h3: properly reject unaligned non-DATA frames
HTTP/3 parser cannot deal with unaligned frames, except for DATA. As it was expected that such case would not occur, a simple BUG_ON() was written to protect HEADERS parsing. First, this BUG_ON() was incorrectly written due an incorrect operator '>=' vs '>' when checking if data wraps. Thus this patch correct it. However, this is not resilient enough, as it still could happen that a large HEADERS frame is unaligned, as HTTP/3 frame header (type + length) is parsed first and removed, which may give some room at the buffer beginning. If this small gap is filled the data will be unaligned. Thus, this patch introduces a new check for non-DATA frames, prior to the parsing functions. If data wraps, the parsing is interrupted and the stream is closed with HTTP/3 error EXCESSIVE LOAD, similarly to a too large frame. This must be backported up to 2.6.
1 parent 2901d0c commit ace7ab7

File tree

1 file changed

+21
-7
lines changed

1 file changed

+21
-7
lines changed

src/h3.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ static ssize_t h3_req_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
641641
/* TODO support trailer parsing in this function */
642642

643643
/* TODO support buffer wrapping */
644-
BUG_ON(b_head(buf) + len >= b_wrap(buf));
644+
BUG_ON(b_head(buf) + len > b_wrap(buf));
645645
ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp,
646646
list, sizeof(list) / sizeof(list[0]));
647647
if (ret < 0) {
@@ -1142,7 +1142,7 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
11421142
TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
11431143

11441144
/* TODO support buffer wrapping */
1145-
BUG_ON(b_head(buf) + len >= b_wrap(buf));
1145+
BUG_ON(b_head(buf) + len > b_wrap(buf));
11461146
ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp,
11471147
list, sizeof(list) / sizeof(list[0]));
11481148
if (ret < 0) {
@@ -1391,7 +1391,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
13911391
TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
13921392

13931393
/* TODO support buffer wrapping */
1394-
BUG_ON(b_head(buf) + len >= b_wrap(buf));
1394+
BUG_ON(b_head(buf) + len > b_wrap(buf));
13951395
ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp,
13961396
list, sizeof(list) / sizeof(list[0]));
13971397
if (ret < 0) {
@@ -1818,10 +1818,11 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
18181818
flen = h3s->demux_frame_len;
18191819
ftype = h3s->demux_frame_type;
18201820

1821-
/* Do not demux incomplete frames except H3 DATA which can be
1822-
* fragmented in multiple HTX blocks.
1821+
/* Current HTTP/3 parser can currently only parse fully
1822+
* received and aligned frames. The only exception is for DATA
1823+
* frames as they can frequently be larger than bufsize.
18231824
*/
1824-
if (flen > b_data(b) && ftype != H3_FT_DATA) {
1825+
if (ftype != H3_FT_DATA) {
18251826
/* Reject frames bigger than bufsize.
18261827
*
18271828
* TODO HEADERS should in complement be limited with H3
@@ -1834,7 +1835,20 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
18341835
qcc_report_glitch(qcs->qcc, 1);
18351836
goto err;
18361837
}
1837-
break;
1838+
1839+
/* TODO extend parser to support the realignment of a frame. */
1840+
if (b_head(b) + b_data(b) > b_wrap(b)) {
1841+
TRACE_ERROR("cannot parse unaligned data frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
1842+
qcc_set_error(qcs->qcc, H3_ERR_EXCESSIVE_LOAD, 1);
1843+
qcc_report_glitch(qcs->qcc, 1);
1844+
goto err;
1845+
}
1846+
1847+
/* Only parse full HTTP/3 frames. */
1848+
if (flen > b_data(b)) {
1849+
TRACE_PROTO("pause parsing on incomplete payload", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
1850+
break;
1851+
}
18381852
}
18391853

18401854
last_stream_frame = (fin && flen == b_data(b));

0 commit comments

Comments
 (0)