mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-23 14:51:27 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MEDIUM: quic: Wrong packet length check in qc_do_rm_hp()

Browse Source 
When entering this function, we first check the packet length is not too short.
But this was done against the datagram lenght in place of the packet length.
This could lead to the header protection to be removed using data past
the end of the packet (without buffer overflow).

Use the packet length in place of the datagram length which is at <end>
address passed as parameter to this function. As the packet length
is already stored in ->len packet struct member, this <end> parameter is no
more useful.

Must be backported to 2.6.
This commit is contained in:
Frédéric Lécaille 2022-08-08 10:28:07 +02:00
parent 87e95d38a9
commit 99897d11d9
1 changed files with 5 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/xprt_quic.c
View File

@ -1240,8 +1240,7 @@ static uint64_t decode_packet_number(uint64_t largest_pn,
*/ */
static int qc_do_rm_hp(struct quic_conn *qc, static int qc_do_rm_hp(struct quic_conn *qc,
struct quic_rx_packet *pkt, struct quic_tls_ctx *tls_ctx, struct quic_rx_packet *pkt, struct quic_tls_ctx *tls_ctx,
int64_t largest_pn, unsigned char *pn, int64_t largest_pn, unsigned char *pn, unsigned char *byte0)
unsigned char *byte0, const unsigned char *end)
{ {
int ret, outlen, i, pnlen; int ret, outlen, i, pnlen;
uint64_t packet_number; uint64_t packet_number;
@ -1252,7 +1251,7 @@ static int qc_do_rm_hp(struct quic_conn *qc,
unsigned char *hp_key; unsigned char *hp_key;
/* Check there is enough data in this packet. */ /* Check there is enough data in this packet. */
if (end - pn < QUIC_PACKET_PN_MAXLEN + sizeof mask) { if (pkt->len - (pn - byte0) < QUIC_PACKET_PN_MAXLEN + sizeof mask) {
TRACE_DEVEL("too short packet", QUIC_EV_CONN_RMHP, qc, pkt); TRACE_DEVEL("too short packet", QUIC_EV_CONN_RMHP, qc, pkt);
return 0; return 0;
} }
@ -3505,8 +3504,7 @@ static inline void qc_rm_hp_pkts(struct quic_conn *qc, struct quic_enc_level *el
tls_ctx = &el->tls_ctx; tls_ctx = &el->tls_ctx;
mt_list_for_each_entry_safe(pqpkt, &el->rx.pqpkts, list, pkttmp1, pkttmp2) { mt_list_for_each_entry_safe(pqpkt, &el->rx.pqpkts, list, pkttmp1, pkttmp2) {
if (!qc_do_rm_hp(qc, pqpkt, tls_ctx, el->pktns->rx.largest_pn, if (!qc_do_rm_hp(qc, pqpkt, tls_ctx, el->pktns->rx.largest_pn,
pqpkt->data + pqpkt->pn_offset, pqpkt->data + pqpkt->pn_offset, pqpkt->data)) {
pqpkt->data, pqpkt->data + pqpkt->len)) {
TRACE_PROTO("hp removing error", QUIC_EV_CONN_ELRMHP, qc); TRACE_PROTO("hp removing error", QUIC_EV_CONN_ELRMHP, qc);
/* XXX TO DO XXX */ /* XXX TO DO XXX */
} }
@ -4598,7 +4596,6 @@ static void qc_pkt_insert(struct quic_rx_packet *pkt, struct quic_enc_level *qel
static inline int qc_try_rm_hp(struct quic_conn *qc, static inline int qc_try_rm_hp(struct quic_conn *qc,
struct quic_rx_packet *pkt, struct quic_rx_packet *pkt,
unsigned char *buf, unsigned char *beg, unsigned char *buf, unsigned char *beg,
const unsigned char *end,
struct quic_enc_level **el) struct quic_enc_level **el)
{ {
unsigned char *pn = NULL; /* Packet number field */ unsigned char *pn = NULL; /* Packet number field */
@ -4624,7 +4621,7 @@ static inline int qc_try_rm_hp(struct quic_conn *qc,
* packets. * packets.
*/ */
if (!qc_do_rm_hp(qc, pkt, &qel->tls_ctx, if (!qc_do_rm_hp(qc, pkt, &qel->tls_ctx,
qel->pktns->rx.largest_pn, pn, beg, end)) { qel->pktns->rx.largest_pn, pn, beg)) {
TRACE_PROTO("hp error", QUIC_EV_CONN_TRMHP, qc); TRACE_PROTO("hp error", QUIC_EV_CONN_TRMHP, qc);
goto err; goto err;
} }
@ -5636,7 +5633,7 @@ static void qc_lstnr_pkt_rcv(unsigned char *buf, const unsigned char *end,
} }
} }
if (!qc_try_rm_hp(qc, pkt, payload, beg, end, &qel)) { if (!qc_try_rm_hp(qc, pkt, payload, beg, &qel)) {
TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT, qc, NULL, NULL, qv); TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT, qc, NULL, NULL, qv);
goto drop; goto drop;
} }