From b49bd199ef55831da689f1478e288bc401e79c47 Mon Sep 17 00:00:00 2001 From: Matthew Szatmary Date: Tue, 19 May 2026 07:46:24 -0700 Subject: [PATCH] [BUG] Reject CRcvBuffer::dropMessage range past buffer end dropMessage early-returns when offset_b < 0 (the entire range is before the buffer start), but does not symmetrically guard against offset_a >= m_szSize (the entire range is past the buffer end). In that case start_off = offset_a is unbounded; incPos() wraps modulo m_szSize and the loop walks legitimate buffer entries, marking them EntryState_Drop -- corrupting receiver state from a single malformed call. Add a symmetric early return for offset_a >= m_szSize. Add CRcvBufferReadMsg.PacketDropLoPastBufferEnd covering the new guard: insert two packets, request a drop whose lo is well past the buffer end, assert no packets were dropped and the buffered packets are still readable. Co-Authored-By: Claude Opus 4.7 (1M context) --- srtcore/buffer_rcv.cpp | 4 ++-- test/test_buffer_rcv.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/srtcore/buffer_rcv.cpp b/srtcore/buffer_rcv.cpp index ae4898eb2..88f94103b 100644 --- a/srtcore/buffer_rcv.cpp +++ b/srtcore/buffer_rcv.cpp @@ -276,10 +276,10 @@ int CRcvBuffer::dropMessage(int32_t seqnolo, int32_t seqnohi, int32_t msgno, Dro // Drop by packet seqno range to also wipe those packets that do not exist in the buffer. const int offset_a = CSeqNo::seqoff(m_iStartSeqNo, seqnolo); const int offset_b = CSeqNo::seqoff(m_iStartSeqNo, seqnohi); - if (offset_b < 0) + if (offset_b < 0 || offset_a >= (int) m_szSize) { LOGC(rbuflog.Debug, log << "CRcvBuffer.dropMessage(): nothing to drop. Requested [" << seqnolo << "; " - << seqnohi << "]. Buffer start " << m_iStartSeqNo << "."); + << seqnohi << "]. Buffer start " << m_iStartSeqNo << " size " << m_szSize << "."); return 0; } diff --git a/test/test_buffer_rcv.cpp b/test/test_buffer_rcv.cpp index 90950911f..5e766de46 100644 --- a/test/test_buffer_rcv.cpp +++ b/test/test_buffer_rcv.cpp @@ -296,6 +296,34 @@ TEST_F(CRcvBufferReadMsg, PacketDropBySeqNo) EXPECT_EQ(m_unit_queue->size(), m_unit_queue->capacity()); } +// Drop ranges whose low seqno is already past the buffer end must be a +// no-op. Without the offset_a >= m_szSize guard, incPos() wraps modulo +// m_szSize and the loop walks legitimate entries -- mirroring the DoS +// shape of a reversed range, just triggered from the other side. +TEST_F(CRcvBufferReadMsg, PacketDropLoPastBufferEnd) +{ + EXPECT_EQ(addMessage(1, 1, m_init_seqno), 0); + EXPECT_EQ(addMessage(1, 2, CSeqNo::incseq(m_init_seqno)), 0); + + auto& rcv_buffer = *m_rcv_buffer.get(); + EXPECT_TRUE(hasAvailablePackets()); + + // Buffer size is m_buff_size_pkts (16); pick a range that begins + // well past the last valid slot. + const int32_t lo = m_init_seqno + 2 * m_buff_size_pkts; + const int32_t hi = lo + 4; + EXPECT_EQ(rcv_buffer.dropMessage(lo, hi, SRT_MSGNO_NONE, CRcvBuffer::DROP_EXISTING), 0); + + // The two packets we inserted must still be present and readable. + EXPECT_TRUE(hasAvailablePackets()); + EXPECT_TRUE(rcv_buffer.isRcvDataReady()); + array buff; + EXPECT_EQ(readMessage(buff.data(), buff.size()), (int) m_payload_sz); + EXPECT_TRUE(verifyPayload(buff.data(), m_payload_sz, m_init_seqno)); + EXPECT_EQ(readMessage(buff.data(), buff.size()), (int) m_payload_sz); + EXPECT_TRUE(verifyPayload(buff.data(), m_payload_sz, CSeqNo::incseq(m_init_seqno))); +} + // Test dropping a message by message number and sequence number. TEST_F(CRcvBufferReadMsg, PacketDropByMsgNoSeqNo) {