Re: [bug report 5/8] Perform initial AEAD validation before creating a channel

Hello Dan,

thank you for taking a look.

On Fri, Apr 10, 2026 at 08:14:50PM +0300, Dan Carpenter wrote:
> Hello Alexandr Nedvedicky,
> 
> Commit c14ae0461352 ("Perform initial AEAD validation before creating
> a channel") from Feb 18, 2025 (linux-next), leads to the following
> Smatch static checker warning:
> 
> 	ssl/quic/quic_port.c:1666 port_default_packet_handler()
> 	error: we previously assumed 'dcid' could be null (see line 1585)
> 
> ssl/quic/quic_port.c
>     1563 static void port_default_packet_handler(QUIC_URXE *e, void *arg,
>     1564     const QUIC_CONN_ID *dcid)
>     1565 {
>     1566     QUIC_PORT *port = arg;
>     1567     PACKET pkt;
>     1568     QUIC_PKT_HDR hdr;
>     1569     QUIC_CHANNEL *ch = NULL, *new_ch = NULL;
>     1570     QUIC_CONN_ID odcid;
>     1571     uint8_t gen_new_token = 0;
>     1572     OSSL_QRX *qrx = NULL;
>     1573     OSSL_QRX *qrx_src = NULL;
>     1574     OSSL_QRX_ARGS qrx_args = { 0 };
>     1575     uint64_t cause_flags = 0;
>     1576     OSSL_QRX_PKT *qrx_pkt = NULL;
>     1577 
>     1578     /* Don't handle anything if we are no longer running. */
>     1579     if (!ossl_quic_port_is_running(port))
>     1580         goto undesirable;
>     1581 
>     1582     if (port_try_handle_stateless_reset(port, e))
>     1583         goto undesirable;
>     1584 
>     1585     if (dcid != NULL
>                  ^^^^^^^^^^^^
> The old code assumed dcid could be NULL

    and yes it can still be in case the caller (demux_process_pending_urxe())
    found malformed header and failed to extract dcid from packet.

> 
>     1586         && ossl_quic_lcidm_lookup(port->lcidm, dcid, NULL,
>     1587             (void **)&ch)) {
>     1588         assert(ch != NULL);
>     1589         ossl_quic_channel_inject(ch, e);
>     1590         return;
>     1591     }
</snip>
>     1610     /*
>     1611      * We set short_conn_id_len to SIZE_MAX here which will cause the decode
>     1612      * operation to fail if we get a 1-RTT packet. This is fine since we only
>     1613      * care about Initial packets.
>     1614      */

    if the packet is malformed the dcid is NULL here. And we know it but the code
    as you are pointing out deliberately ignores that warning sign and is going
    to parse the malformed packet.

>     1615     if (!ossl_quic_wire_decode_pkt_hdr(&pkt, SIZE_MAX, 1, 0, &hdr, NULL,
>     1616             &cause_flags)) {
>     1617         /*
>     1618          * If we fail due to a bad version, we know the packet up to the version
>     1619          * number was decoded, and we use it below to send a version
>     1620          * negotiation packet
>     1621          */
>     1622         if ((cause_flags & QUIC_PKT_HDR_DECODE_BAD_VERSION) == 0)
>     1623             goto undesirable;

    the goto at line 1623 avoids that NULL pointer dereference, however I agree we
    could let function to exit way earlier saving CPU cycles on futile processing
    of malformed packet.

>     1624     }
>     1625 
>     1626     switch (hdr.version) {
>     1627     case QUIC_VERSION_1:
>     1628         break;
>     1629 

</snip>
>     1664     qrx_args.libctx = port->engine->libctx;
>     1665     qrx_args.demux = port->demux;
> --> 1666     qrx_args.short_conn_id_len = dcid->id_len;
>                                           ^^^^^^^^^^^^
> but the patch adds an unchecked dereference

    fortunately the code just burns CPU cycles.


I think adding a dcid == NULL check somewhere as we enter the function
and let function to fail when dcid is not found is worth thing to do.

Are you interested to submit PR? or shall I proceed with PR?

thanks and
regards
sashan

-- 
You received this message because you are subscribed to the Google Groups "openssl-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openssl-users+unsubscribe-MCmKBN63+Bmbup2nOX2J7Q@public.gmane.org
To view this discussion visit https://groups.google.com/a/openssl.org/d/msgid/openssl-users/ady-HDKgsS__5hvJ%40lifty.flatnet.local.