DTLS handshake qsort bug
Hi all,
I believe that I have discovered an issue in the DTLS handshake receive buffering logic in lib/buffers.c where the comparator passed to qsort() does not correctly handle equal sequence numbers.
In handshake_compare(), the comparator returns 1 whenever e1->sequence <= e2->sequence, and -1 otherwise:
static int handshake_compare(const void *_e1, const void *_e2)
{
const handshake_buffer_st *e1 = _e1;
const handshake_buffer_st *e2 = _e2;
if (e1->sequence <= e2->sequence)
return 1;
else
return -1;
}This comparator is then used when sorting buffered DTLS handshake messages:
if (session->internals.handshake_recv_buffer_size > 1)
qsort(recv_buf,
session->internals.handshake_recv_buffer_size,
sizeof(recv_buf[0]), handshake_compare);The issue is that the comparator never returns 0 when e1->sequence == e2->sequence. As a result, it violates the contract expected by qsort(), which requires equal elements to compare equal.
In DTLS, equal message_seq values appear to be possible in practice because multiple fragments of the same handshake message share the same sequence number, and retransmissions also reuse the same sequence number. If two buffered entries with the same sequence are present when sorting occurs, qsort() is invoked with a comparator that does not define equality for those entries.
This can lead to undefined behavior during sorting. At minimum, it seems to permit inconsistent or unstable ordering of buffered handshake messages.
Proposed minimal fix:
diff --git a/lib/buffers.c b/lib/buffers.c
@@
static int handshake_compare(const void *_e1, const void *_e2)
{
const handshake_buffer_st *e1 = _e1;
const handshake_buffer_st *e2 = _e2;
- if (e1->sequence <= e2->sequence)
+ if (e1->sequence < e2->sequence)
return 1;
- else
+ else if (e1->sequence > e2->sequence)
return -1;
+ else
+ return 0;
}This preserves the existing ordering direction while making the comparator consistent for equal sequence numbers.
Cheers, Josh