From: Frédéric Lécaille <flecaille@haproxy.com>
Date: Tue, 23 Aug 2022 15:40:09 +0000 (+0200)
Subject: BUG/MINOR: quic: Safer QUIC frame builders
X-Git-Tag: v2.7-dev5~72
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b8047de11a4a4083a9fc92bf2bd4b731163fc149;p=thirdparty%2Fhaproxy.git

BUG/MINOR: quic: Safer QUIC frame builders

Do not rely on the fact the callers of qc_build_frm() handle their
buffer passed to function the correct way (without leaving garbage).
Make qc_build_frm() update the buffer passed as argument only if
the frame it builds is well formed.

As far as I sse, there is no such callers which does not handle
carefully such buffers.

Must be backported to 2.6.
---

diff --git a/src/quic_frame.c b/src/quic_frame.c
index f590be2922..58e9ebcdcf 100644
--- a/src/quic_frame.c
+++ b/src/quic_frame.c
@@ -1122,6 +1122,8 @@ int qc_parse_frm(struct quic_frame *frm, struct quic_rx_packet *pkt,
 
 /* Encode <frm> QUIC frame into <buf> buffer.
  * Returns 1 if succeeded (enough room in <buf> to encode the frame), 0 if not.
+ * The buffer is updated to point to one byte past the end of the built frame
+ * only if succeeded.
  */
 int qc_build_frm(unsigned char **buf, const unsigned char *end,
                  struct quic_frame *frm, struct quic_tx_packet *pkt,
@@ -1129,6 +1131,7 @@ int qc_build_frm(unsigned char **buf, const unsigned char *end,
 {
 	int ret = 0;
 	const struct quic_frame_builder *builder;
+	unsigned char *pos = *buf;
 
 	TRACE_ENTER(QUIC_EV_CONN_BFRM, qc);
 	builder = &quic_frame_builders[frm->type];
@@ -1138,19 +1141,20 @@ int qc_build_frm(unsigned char **buf, const unsigned char *end,
 		BUG_ON(!(builder->mask & (1U << pkt->type)));
 	}
 
-	if (end <= *buf) {
+	if (end <= pos) {
 		TRACE_DEVEL("not enough room", QUIC_EV_CONN_BFRM, qc, frm);
 		goto leave;
 	}
 
 	TRACE_PROTO("frame", QUIC_EV_CONN_BFRM, qc, frm);
-	*(*buf)++ = frm->type;
-	if (!quic_frame_builders[frm->type].func(buf, end, frm, qc)) {
+	*pos++ = frm->type;
+	if (!quic_frame_builders[frm->type].func(&pos, end, frm, qc)) {
 		TRACE_DEVEL("frame building error", QUIC_EV_CONN_BFRM, qc, frm);
 		goto leave;
 	}
 
 	pkt->flags |= builder->flags;
+	*buf = pos;
 
 	ret = 1;
  leave:
diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index 4f2a2ea050..0e9e46e7c7 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -6832,9 +6832,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
 	if (!LIST_ISEMPTY(&frm_list)) {
 		struct quic_frame *tmp_cf;
 		list_for_each_entry_safe(cf, tmp_cf, &frm_list, list) {
-			unsigned char *spos = pos;
-
-			if (!qc_build_frm(&spos, end, cf, pkt, qc)) {
+			if (!qc_build_frm(&pos, end, cf, pkt, qc)) {
 				ssize_t room = end - pos;
 				TRACE_DEVEL("Not enough room", QUIC_EV_CONN_TXPKT,
 				            qc, NULL, NULL, &room);
@@ -6848,7 +6846,6 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
 				break;
 			}
 
-			pos = spos;
 			quic_tx_packet_refinc(pkt);
 			cf->pkt = pkt;
 		}