BUG/MEDIUM: quic: Possible use of uninitialized <odcid> variable in qc_lstnr_params_init()
When receiving a token into a client Initial packet without a cluster secret defined
by configuration, the <odcid> variable used to parse the ODCID from the token
could be used without having been initialized. Such a packet must be dropped. So
the sufficient part of this patch is this check:
+ }
+ else if (!global.cluster_secret && token_len) {
+ /* Impossible case: a token was received without configured
+ * cluster secret.
+ */
+ TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT,
+ NULL, NULL, NULL, qv);
+ goto drop;
}
Take the opportunity of this patch to rework and make it more readable this part
of code where such a packet must be dropped removing the <check_token> variable.
When an ODCID is parsed from a token, new <token_odcid> new pointer variable
is set to the address of the parsed ODCID. This way, is not set but used it will
make crash haproxy. This was not always the case with an uninitialized local
variable.
Adapt the API to used such a pointer variable: <token> boolean variable is removed
from qc_lstnr_params_init() prototype.
This must be backported to 2.6.
diff --git a/src/quic_tp.c b/src/quic_tp.c
index f83ed86..449f94e 100644
--- a/src/quic_tp.c
+++ b/src/quic_tp.c
@@ -634,8 +634,8 @@
/* QUIC server (or haproxy listener) only function.
* Initialize the local transport parameters <rx_params> from <listener_params>
* coming from configuration and Initial packet information (destintation
- * connection ID, source connection ID, original destination connection ID,
- * and if a token was present denoted by <token> boolean value.
+ * connection ID, source connection ID, original destination connection ID from
+ * client token.
* Returns 1 if succeeded, 0 if not.
*/
int qc_lstnr_params_init(struct quic_conn *qc,
@@ -643,7 +643,7 @@
const unsigned char *stateless_reset_token,
const unsigned char *dcid, size_t dcidlen,
const unsigned char *scid, size_t scidlen,
- const unsigned char *odcid, size_t odcidlen, int token)
+ const unsigned char *token_odcid, size_t token_odcidlen)
{
struct quic_transport_params *rx_params = &qc->rx.params;
struct tp_cid *odcid_param = &rx_params->original_destination_connection_id;
@@ -654,9 +654,9 @@
memcpy(rx_params->stateless_reset_token, stateless_reset_token,
sizeof rx_params->stateless_reset_token);
/* Copy original_destination_connection_id transport parameter. */
- if (token) {
- memcpy(odcid_param->data, odcid, odcidlen);
- odcid_param->len = odcidlen;
+ if (token_odcid) {
+ memcpy(odcid_param->data, token_odcid, token_odcidlen);
+ odcid_param->len = token_odcidlen;
/* Copy retry_source_connection_id transport parameter. */
memcpy(rx_params->retry_source_connection_id.data, dcid, dcidlen);
rx_params->retry_source_connection_id.len = dcidlen;