mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-02-20 04:37:04 +00:00
BUG/MINOR: quic: prevent buggy memcpy for empty STREAM
Sometimes it may be necessary to send empty STREAM frames with only the FIN bit set. For these frames, memcpy is thus unnecessary as their payload is empty. However, we did not prevent its invocation inside quic_build_stream_frame(). Normally, memcpy invocation with length==0 is safe. However, there is an extra condition in our function to handle data wrapping. For an empty STREAM frame in the context of MUX emission, this is safe as the frame points to a valid buffer which causes the wrapping condition to be false and resulting in a memcpy with 0 length. However, in the context of retransmission, this may lead to a crash. Consider the following scenario : two STREAM frames A and B are produced, one with payload and one empty with FIN set, pointing to the same stream_desc buffer. If A is acknowledged by the peer, its buffer is released as no more data is left in it. If B needs to be resent, the wrapping condition will be messed up to a reuse of a freed buffer. Most of the times, <wrap> will be a negative number, which results in a memcpy invocation causing a buffer overflow. To fix this, simply add an extra condition to skip memcpy and wrapping check if STREAM frame length is null inside quic_build_stream_frame(). This crash is pretty rare as it relies on a lot of conditions difficult to reproduce. It seems to be the cause for the latest crashes reported under github issue #2120. In all the inspected dumps, the segfault occurred during retransmission with an empty STREAM frame being used as input. Thanks again to Tristan from Mangadex for his help and investigation on it. This should be backported up to 2.6.
This commit is contained in:
parent
7c5591facb
commit
19eaf88fda
@ -520,6 +520,10 @@ static int quic_build_stream_frame(unsigned char **buf, const unsigned char *end
|
||||
(!quic_enc_int(buf, end, strm_frm->len) || end - *buf < strm_frm->len)))
|
||||
return 0;
|
||||
|
||||
/* No need for data memcpy if no payload. */
|
||||
if (!strm_frm->len)
|
||||
return 1;
|
||||
|
||||
wrap = (const unsigned char *)b_wrap(strm_frm->buf);
|
||||
if (strm_frm->data + strm_frm->len > wrap) {
|
||||
size_t to_copy = wrap - strm_frm->data;
|
||||
|
Loading…
Reference in New Issue
Block a user