From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Mon, 25 Apr 2022 12:26:54 +0000 (+0200)
Subject: BUG/MINOR: quic: fix use-after-free with trace on ACK consume
X-Git-Tag: v2.6-dev8~91
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7586bef6d7fc866659aff9017a644d7f48ba16ce;p=thirdparty%2Fhaproxy.git

BUG/MINOR: quic: fix use-after-free with trace on ACK consume

When using qc_stream_desc_ack(), the stream instance may be freed if
there is no more data in its buffers. This also means that all frames
still stored waiting for ACK for this stream are freed via
qc_stream_desc_free().

This is particularly important in quic_stream_try_to_consume() where we
loop over the frames tree of the stream. A use-after-free is present in
cas the stream has been freed in the trace "stream consumed" which
dereference the frame. Fix this by first checking if the stream has been
freed or not.

This bug was detected by using ASAN + quic traces enabled.
---

diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index c1a7be1a9c..7f331d1692 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -1454,11 +1454,16 @@ static int quic_stream_try_to_consume(struct quic_conn *qc,
 			break;
 
 		if (qc_stream_desc_ack(&stream, offset, len)) {
+			/* cf. next comment : frame may be freed at this stage. */
 			TRACE_PROTO("stream consumed", QUIC_EV_CONN_ACKSTRM,
-			            qc, strm, stream);
+			            qc, stream ? strm : NULL, stream);
 			ret = 1;
 		}
 
+		/* If stream is NULL after qc_stream_desc_ack(), it means frame
+		 * has been freed. with the stream frames tree. Nothing to do
+		 * anymore in here.
+		 */
 		if (!stream)
 			return 1;