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

Hello Dan,

On Mon, Apr 13, 2026 at 01:07:47PM +0300, Dan Carpenter wrote:
</snip>
> > 
> >     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.
> 
> Huh.  It's a bit complicated for me to change the static checker to
> connect the dots between a NULL dcid and the goto on line 1623...
> Thanks for taking a look!

    your static analyzer is doing a good job. it identified the
    place with suspicious smell in code. what else one can ask for.

> 
> > 
> > >     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?
> 
> I'm trying to get out of the business of patching the code myself.
> I'd like to extend static analysis to all the C programs in Debian
> but I'm having a hard time scaling up the bug reporting process...
> 

    I will submit PR then with all the credit you deserve.

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/adzGIoXi95svDp6T%40lifty.flatnet.local.