QUIC RX: Refactor unsafe DCID consistency checking

Previously, we enforced the requirement that the DCIDs be the same for
all packets in a datagram by keeping a pointer to the first RXE
generated from a datagram. This is unsafe and could lead to a UAF if the
first packet is malformed, meaning that no RXE ended up being generated
from it. Keep track of the DCID directly instead, as we should enforce
this correctly even if the first packet in a datagram is malformed (but
has an intelligible header with a DCID and length).

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19703)

1 files changed, 12 insertions, 13 deletions

diff --git a/test/quic_record_test.c b/test/quic_record_test.c
index 1664cd2..cd83f66 100644
--- a/test/quic_record_test.c
+++ b/test/quic_record_test.c
@@ -1710,12 +1710,6 @@ static void rx_state_teardown(struct rx_state *s)
         s->ccdata = NULL;
     }
 
-    if (s->qrx != NULL) {
-        ossl_qrx_free(s->qrx);
-        ossl_quic_conn_set_qrx(s->quic_conn, NULL);
-        s->qrx = NULL;
-    }
-
     if (s->quic_conn != NULL) {
         SSL_free((SSL *)s->quic_conn);
         s->quic_conn = NULL;
@@ -1725,6 +1719,11 @@ static void rx_state_teardown(struct rx_state *s)
         s->quic_ssl_ctx = NULL;
     }
 
+    if (s->qrx != NULL) {
+        ossl_qrx_free(s->qrx);
+        s->qrx = NULL;
+    }
+
     if (s->demux != NULL) {
         ossl_quic_demux_free(s->demux);
         s->demux = NULL;
@@ -1800,7 +1799,7 @@ static int rx_state_ensure_for_frames(struct rx_state *s)
 
 static int rx_run_script(const struct rx_test_op *script)
 {
-    int testresult = 0, pkt_outstanding = 0;
+    int testresult = 0;
     struct rx_state s = {0};
     size_t i;
     OSSL_QRX_PKT *pkt = NULL;
@@ -1861,7 +1860,6 @@ static int rx_run_script(const struct rx_test_op *script)
                 if (!TEST_true(ossl_qrx_read_pkt(s.qrx, &pkt)))
                     goto err;
 
-                pkt_outstanding = 1;
                 if (!TEST_ptr(pkt) || !TEST_ptr(pkt->hdr))
                     goto err;
 
@@ -1877,20 +1875,22 @@ static int rx_run_script(const struct rx_test_op *script)
                 case RX_TEST_OP_CHECK_PKT_FRAMES_OK:
                     if (!TEST_true(rx_state_ensure_for_frames(&s)))
                         goto err;
-                    pkt_outstanding = 0;
                     if (!TEST_true(ossl_quic_handle_frames(s.quic_conn, pkt)))
                         goto err;
+                    ossl_qrx_pkt_release(pkt);
+                    pkt = NULL;
                     break;
                 case RX_TEST_OP_CHECK_PKT_FRAMES_INVALID:
                     if (!TEST_true(rx_state_ensure_for_frames(&s)))
                         goto err;
-                    pkt_outstanding = 0;
                     if (!TEST_false(ossl_quic_handle_frames(s.quic_conn, pkt)))
                         goto err;
+                    ossl_qrx_pkt_release(pkt);
+                    pkt = NULL;
                     break;
                 default:
-                    pkt_outstanding = 0;
                     ossl_qrx_pkt_release(pkt);
+                    pkt = NULL;
                     break;
                 }
                 break;
@@ -1931,8 +1931,7 @@ static int rx_run_script(const struct rx_test_op *script)
 
     testresult = 1;
 err:
-    if (pkt_outstanding)
-        ossl_qrx_pkt_release(pkt);
+    ossl_qrx_pkt_release(pkt);
     rx_state_teardown(&s);
     return testresult;
 }