BUG/MINOR: quic: Possible leak when allocating an encryption level

This bug was reported by GH #2200 (coverity issue) as follows:

*** CID 1516590:  Resource leaks  (RESOURCE_LEAK)
/src/quic_tls.c: 159 in quic_conn_enc_level_init()
153
154             LIST_APPEND(&qc->qel_list, &qel->list);
155             *el = qel;
156             ret = 1;
157      leave:
158             TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
>>>     CID 1516590:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "qel" going out of scope leaks the storage it points to.
159             return ret;
160     }
161
162     /* Uninitialize <qel> QUIC encryption level. Never fails. */
163     void quic_conn_enc_level_uninit(struct quic_conn *qc, struct quic_enc_level *qel)
164     {

This bug was introduced by this commit which has foolishly assumed the encryption
level memory would be released after quic_conn_enc_level_init() has failed. This
is no more possible because this object is dynamic and no more a static member
of the QUIC connection object.

Anyway, this patch modifies quic_conn_enc_level_init() to ensure this is
no more leak when quic_conn_enc_level_init() fails calling quic_conn_enc_level_uninit()
in case of memory allocation error.

quic_conn_enc_level_uninit() code was moved without modification only to be defined
before quic_conn_enc_level_init()

There is no need to backport this.
This commit is contained in:
Frédéric Lécaille 2023-07-03 10:28:33 +02:00
parent fdc57c4021
commit 0e53cb07a5

View File

@ -100,6 +100,25 @@ void quic_tls_secret_hexdump(struct buffer *buf,
chunk_appendf(buf, "%02x", secret[i]);
}
/* Uninitialize <qel> QUIC encryption level. Never fails. */
void quic_conn_enc_level_uninit(struct quic_conn *qc, struct quic_enc_level *qel)
{
int i;
TRACE_ENTER(QUIC_EV_CONN_CLOSE, qc);
for (i = 0; i < qel->tx.crypto.nb_buf; i++) {
if (qel->tx.crypto.bufs[i]) {
pool_free(pool_head_quic_crypto_buf, qel->tx.crypto.bufs[i]);
qel->tx.crypto.bufs[i] = NULL;
}
}
ha_free(&qel->tx.crypto.bufs);
quic_cstream_free(qel->cstream);
TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
}
/* Initialize QUIC TLS encryption level with <level<> as level for <qc> QUIC
* connection allocating everything needed.
*
@ -120,6 +139,9 @@ static int quic_conn_enc_level_init(struct quic_conn *qc,
if (!qel)
goto leave;
qel->tx.crypto.bufs = NULL;
qel->tx.crypto.nb_buf = 0;
qel->cstream = NULL;
qel->pktns = pktns;
qel->level = level;
quic_tls_ctx_reset(&qel->tls_ctx);
@ -131,11 +153,12 @@ static int quic_conn_enc_level_init(struct quic_conn *qc,
/* TODO: use a pool */
qel->tx.crypto.bufs = malloc(sizeof *qel->tx.crypto.bufs);
if (!qel->tx.crypto.bufs)
goto leave;
goto err;
qel->tx.crypto.bufs[0] = pool_alloc(pool_head_quic_crypto_buf);
if (!qel->tx.crypto.bufs[0])
goto leave;
goto err;
qel->tx.crypto.bufs[0]->sz = 0;
qel->tx.crypto.nb_buf = 1;
@ -148,7 +171,7 @@ static int quic_conn_enc_level_init(struct quic_conn *qc,
else {
qel->cstream = quic_cstream_new(qc);
if (!qel->cstream)
goto leave;
goto err;
}
LIST_APPEND(&qc->qel_list, &qel->list);
@ -157,25 +180,11 @@ static int quic_conn_enc_level_init(struct quic_conn *qc,
leave:
TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
return ret;
}
/* Uninitialize <qel> QUIC encryption level. Never fails. */
void quic_conn_enc_level_uninit(struct quic_conn *qc, struct quic_enc_level *qel)
{
int i;
TRACE_ENTER(QUIC_EV_CONN_CLOSE, qc);
for (i = 0; i < qel->tx.crypto.nb_buf; i++) {
if (qel->tx.crypto.bufs[i]) {
pool_free(pool_head_quic_crypto_buf, qel->tx.crypto.bufs[i]);
qel->tx.crypto.bufs[i] = NULL;
}
}
ha_free(&qel->tx.crypto.bufs);
quic_cstream_free(qel->cstream);
TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
err:
quic_conn_enc_level_uninit(qc, qel);
pool_free(pool_head_quic_enc_level, qel);
goto leave;
}
/* Allocate a QUIC TLS encryption with <level> as TLS stack encryption to be