BUG/MINOR: quic: Possible crash when dumping version information

->others member of tp_version_information structure pointed to a buffer in the
TLS stack used to parse the transport parameters. There is no garantee that this
buffer is available until the connection is released.

Do not dump the available versions selected by the client anymore, but displayed the
chosen one (selected by the client for this connection) and the negotiated one.

Must be backported to 2.7 and 2.6.
This commit is contained in:
Frédéric Lécaille 2023-05-10 13:06:31 +02:00
parent c147171d57
commit b971696296
3 changed files with 7 additions and 22 deletions

View File

@ -28,8 +28,6 @@ struct tp_preferred_address {
struct tp_version_information { struct tp_version_information {
uint32_t chosen; uint32_t chosen;
const uint32_t *others;
size_t nb_others;
const struct quic_version *negotiated_version; const struct quic_version *negotiated_version;
}; };

View File

@ -45,21 +45,9 @@ static inline void quic_tp_version_info_dump(struct buffer *b,
if (!tp->chosen) if (!tp->chosen)
return; return;
chunk_appendf(b, " version_information:(chosen=0x%08x", tp->chosen); chunk_appendf(b, " versions:chosen=0x%08x", tp->chosen);
if (tp->nb_others) { if (tp->negotiated_version)
int i = 0; chunk_appendf(b, ",negotiated=0x%08x", tp->negotiated_version->num);
const uint32_t *ver;
chunk_appendf(b, ",others=");
for (ver = tp->others; i < tp->nb_others; i++, ver++) {
if (i != 0)
chunk_appendf(b, ",");
if (local)
chunk_appendf(b, "0x%08x", *ver);
else
chunk_appendf(b, "0x%08x", ntohl(*ver));
}
}
chunk_appendf(b, ")");
} }
static inline void quic_transport_params_dump(struct buffer *b, static inline void quic_transport_params_dump(struct buffer *b,

View File

@ -172,7 +172,7 @@ static int quic_transport_param_dec_version_info(struct tp_version_information *
const unsigned char *end, int server) const unsigned char *end, int server)
{ {
size_t tp_len = end - *buf; size_t tp_len = end - *buf;
const uint32_t *ver; const uint32_t *ver, *others;
/* <tp_len> must be a multiple of sizeof(uint32_t) */ /* <tp_len> must be a multiple of sizeof(uint32_t) */
if (tp_len < sizeof tp->chosen || (tp_len & 0x3)) if (tp_len < sizeof tp->chosen || (tp_len & 0x3))
@ -184,10 +184,10 @@ static int quic_transport_param_dec_version_info(struct tp_version_information *
return 0; return 0;
*buf += sizeof tp->chosen; *buf += sizeof tp->chosen;
tp->others = (const uint32_t *)*buf; others = (const uint32_t *)*buf;
/* Others versions must not be null */ /* Others versions must not be null */
for (ver = tp->others; ver < (const uint32_t *)end; ver++) { for (ver = others; ver < (const uint32_t *)end; ver++) {
if (!*ver) if (!*ver)
return 0; return 0;
} }
@ -196,8 +196,7 @@ static int quic_transport_param_dec_version_info(struct tp_version_information *
/* TODO: not supported */ /* TODO: not supported */
return 0; return 0;
tp->nb_others = (end - (const unsigned char *)tp->others) / sizeof *tp->others; for (ver = others; ver < (const uint32_t *)end; ver++) {
for (ver = tp->others; ver < (const uint32_t *)end; ver++) {
if (!tp->negotiated_version) { if (!tp->negotiated_version) {
int i; int i;