Delayed packets in mini-connection reference volatile memory, leading to data corruption

[sabitov-kirill]

Hello, team! First of all thanks for great job on lsquic!

Problem description

Backstory. In our very specific scenario we use both server side and client side certificates. Thus handshake packets (from client to server) are much larger then usual. This creates edge-case situation where we encountered implementation flow.

Actual problem. When lsquic receives a packet on a mini-connection (IETF QUIC) that cannot be decrypted yet (e.g., due to missing keys during the handshake), the packet is delayed for later processing. However, the library fails to create a copy of the packet's payload data. Instead, the delayed lsquic_packet_in structure retains a pointer to the original buffer provided by the caller of lsquic_engine_packet_in.

If the caller (the application integrating lsquic) reuses or frees this buffer after lsquic_engine_packet_in returns—which is standard behavior—the delayed packet's data becomes corrupted. When the connection eventually transitions to a full connection and attempts to process the delayed queue, it reads garbage data or crashes.

Expected Behavior: When a packet is delayed, lsquic should allocate a new buffer, copy the packet payload into it, update packet_in->pi_data, and set the PI_OWN_DATA flag to ensure the memory is managed internally and persists until the packet is processed.

Root Cause Analysis

In lsquic_mini_conn_ietf.c, the function imico_maybe_delay_processing queues the packet but does not ensure the data is owned by the library:

static void
imico_maybe_delay_processing (struct ietf_mini_conn *conn,
                                            struct lsquic_packet_in *packet_in)
{
    // ... checks ...
    if (conn->imc_delayed_packets_count < max_delayed)
    {
        ++conn->imc_delayed_packets_count;
        lsquic_packet_in_upref(packet_in);
        TAILQ_INSERT_TAIL(&conn->imc_app_packets, packet_in, pi_next);        
        // No copy of packet_in->pi_data is done
        // ...
    }
    // ...
}

Notes:
I have prepared PR (#594), that proposes new unit test for this specific case and fixes described flaw.