mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-20 21:31:28 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: quic: Safer QUIC frame builders

Browse Source 
Do not rely on the fact the callers of qc_build_frm() handle their
buffer passed to function the correct way (without leaving garbage).
Make qc_build_frm() update the buffer passed as argument only if
the frame it builds is well formed.

As far as I sse, there is no such callers which does not handle
carefully such buffers.

Must be backported to 2.6.
This commit is contained in:
Frédéric Lécaille 2022-08-23 17:40:09 +02:00
parent a8a6043240
commit b8047de11a
2 changed files with 8 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/quic_frame.c
View File

@ -1122,6 +1122,8 @@ int qc_parse_frm(struct quic_frame *frm, struct quic_rx_packet *pkt,
/* Encode <frm> QUIC frame into <buf> buffer. /* Encode <frm> QUIC frame into <buf> buffer.
* Returns 1 if succeeded (enough room in <buf> to encode the frame), 0 if not. * Returns 1 if succeeded (enough room in <buf> to encode the frame), 0 if not.
* The buffer is updated to point to one byte past the end of the built frame
* only if succeeded.
*/ */
int qc_build_frm(unsigned char **buf, const unsigned char *end, int qc_build_frm(unsigned char **buf, const unsigned char *end,
struct quic_frame *frm, struct quic_tx_packet *pkt, struct quic_frame *frm, struct quic_tx_packet *pkt,
@ -1129,6 +1131,7 @@ int qc_build_frm(unsigned char **buf, const unsigned char *end,
{ {
int ret = 0; int ret = 0;
const struct quic_frame_builder *builder; const struct quic_frame_builder *builder;
unsigned char *pos = *buf;
TRACE_ENTER(QUIC_EV_CONN_BFRM, qc); TRACE_ENTER(QUIC_EV_CONN_BFRM, qc);
builder = &quic_frame_builders[frm->type]; builder = &quic_frame_builders[frm->type];
@ -1138,19 +1141,20 @@ int qc_build_frm(unsigned char **buf, const unsigned char *end,
BUG_ON(!(builder->mask & (1U << pkt->type))); BUG_ON(!(builder->mask & (1U << pkt->type)));
} }
if (end <= *buf) { if (end <= pos) {
TRACE_DEVEL("not enough room", QUIC_EV_CONN_BFRM, qc, frm); TRACE_DEVEL("not enough room", QUIC_EV_CONN_BFRM, qc, frm);
goto leave; goto leave;
} }
TRACE_PROTO("frame", QUIC_EV_CONN_BFRM, qc, frm); TRACE_PROTO("frame", QUIC_EV_CONN_BFRM, qc, frm);
*(*buf)++ = frm->type; *pos++ = frm->type;
if (!quic_frame_builders[frm->type].func(buf, end, frm, qc)) { if (!quic_frame_builders[frm->type].func(&pos, end, frm, qc)) {
TRACE_DEVEL("frame building error", QUIC_EV_CONN_BFRM, qc, frm); TRACE_DEVEL("frame building error", QUIC_EV_CONN_BFRM, qc, frm);
goto leave; goto leave;
} }
pkt->flags |= builder->flags; pkt->flags |= builder->flags;
*buf = pos;
ret = 1; ret = 1;
leave: leave:

5
src/xprt_quic.c
View File

@ -6832,9 +6832,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
if (!LIST_ISEMPTY(&frm_list)) { if (!LIST_ISEMPTY(&frm_list)) {
struct quic_frame *tmp_cf; struct quic_frame *tmp_cf;
list_for_each_entry_safe(cf, tmp_cf, &frm_list, list) { list_for_each_entry_safe(cf, tmp_cf, &frm_list, list) {
unsigned char *spos = pos; if (!qc_build_frm(&pos, end, cf, pkt, qc)) {
if (!qc_build_frm(&spos, end, cf, pkt, qc)) {
ssize_t room = end - pos; ssize_t room = end - pos;
TRACE_DEVEL("Not enough room", QUIC_EV_CONN_TXPKT, TRACE_DEVEL("Not enough room", QUIC_EV_CONN_TXPKT,
qc, NULL, NULL, &room); qc, NULL, NULL, &room);
@ -6848,7 +6846,6 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
break; break;
} }
pos = spos;
quic_tx_packet_refinc(pkt); quic_tx_packet_refinc(pkt);
cf->pkt = pkt; cf->pkt = pkt;
} }