BUG/MEDIUM: quic: CRYPTO frame freeing without eb_delete()

Since this commit:

	BUG/MINOR: quic: reorder fragmented RX CRYPTO frames by their offsets

when they are parsed, the CRYPTO frames are ordered by their offsets into an ebtree.
Then their data are provided to the ncbufs.

But in case of error, when qc_handle_crypto_frm() returns QUIC_RX_RET_FRM_FATAL
or QUIC_RX_RET_FRM_AGAIN), they remain attached to their tree. Then
from <err> label, they are deteleted and deleted (with a while(node) { eb_delete();
qc_frm_free();} loop). But before this loop, these statements directly
free the frame without deleting it from its tree, if this is a CRYPTO frame,
leading to a use after free when running the loop:

     if (frm)
	    qc_frm_free(qc, &frm);

This issue was detected by the interop tests, with quic-go as client. Weirdly, this
client sends CRYPTO frames by packet with holes.
Must be backported as far as 2.6 as the commit mentioned above.

(cherry picked from commit 800ba73a9ca39dc0e0c33230c87ececd540f5541)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit ed1b2dc1f8ec5a68dbf3e075d4bffd5bc738bf2b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit e3b7cd5d61b6ea1c1b42ddcd5539e1fdf9cfe542)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/quic_rx.c b/src/quic_rx.c
index 5bf7ec2..96c3be0 100644 (file)
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -1095,10 +1095,14 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                ret = qc_handle_crypto_frm(qc, &frm->crypto, pkt, qel);
                switch (ret) {
                case QUIC_RX_RET_FRM_FATAL:
+                       /* avoid freeing without eb_delete() */
+                       frm = NULL;
                        goto err;
 
                case QUIC_RX_RET_FRM_AGAIN:
                        TRACE_STATE("AGAIN encountered", QUIC_EV_CONN_PRSHPKT, qc);
+                       /* avoid freeing without eb_delete() */
+                       frm = NULL;
                        goto err;
 
                case QUIC_RX_RET_FRM_DONE: