Skip to content

Commit 2e2ab20

Browse files
[BUG] Reject CRcvBuffer::dropMessage range past buffer end (#3322)
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) <noreply@anthropic.com>
1 parent 033d1d5 commit 2e2ab20

2 files changed

Lines changed: 30 additions & 2 deletions

File tree

srtcore/buffer_rcv.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,10 @@ int CRcvBuffer::dropMessage(int32_t seqnolo, int32_t seqnohi, int32_t msgno, Dro
276276
// Drop by packet seqno range to also wipe those packets that do not exist in the buffer.
277277
const int offset_a = CSeqNo::seqoff(m_iStartSeqNo, seqnolo);
278278
const int offset_b = CSeqNo::seqoff(m_iStartSeqNo, seqnohi);
279-
if (offset_b < 0)
279+
if (offset_b < 0 || offset_a >= (int) m_szSize)
280280
{
281281
LOGC(rbuflog.Debug, log << "CRcvBuffer.dropMessage(): nothing to drop. Requested [" << seqnolo << "; "
282-
<< seqnohi << "]. Buffer start " << m_iStartSeqNo << ".");
282+
<< seqnohi << "]. Buffer start " << m_iStartSeqNo << " size " << m_szSize << ".");
283283
return 0;
284284
}
285285

test/test_buffer_rcv.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,34 @@ TEST_F(CRcvBufferReadMsg, PacketDropBySeqNo)
296296
EXPECT_EQ(m_unit_queue->size(), m_unit_queue->capacity());
297297
}
298298

299+
// Drop ranges whose low seqno is already past the buffer end must be a
300+
// no-op. Without the offset_a >= m_szSize guard, incPos() wraps modulo
301+
// m_szSize and the loop walks legitimate entries -- mirroring the DoS
302+
// shape of a reversed range, just triggered from the other side.
303+
TEST_F(CRcvBufferReadMsg, PacketDropLoPastBufferEnd)
304+
{
305+
EXPECT_EQ(addMessage(1, 1, m_init_seqno), 0);
306+
EXPECT_EQ(addMessage(1, 2, CSeqNo::incseq(m_init_seqno)), 0);
307+
308+
auto& rcv_buffer = *m_rcv_buffer.get();
309+
EXPECT_TRUE(hasAvailablePackets());
310+
311+
// Buffer size is m_buff_size_pkts (16); pick a range that begins
312+
// well past the last valid slot.
313+
const int32_t lo = m_init_seqno + 2 * m_buff_size_pkts;
314+
const int32_t hi = lo + 4;
315+
EXPECT_EQ(rcv_buffer.dropMessage(lo, hi, SRT_MSGNO_NONE, CRcvBuffer::DROP_EXISTING), 0);
316+
317+
// The two packets we inserted must still be present and readable.
318+
EXPECT_TRUE(hasAvailablePackets());
319+
EXPECT_TRUE(rcv_buffer.isRcvDataReady());
320+
array<char, m_payload_sz> buff;
321+
EXPECT_EQ(readMessage(buff.data(), buff.size()), (int) m_payload_sz);
322+
EXPECT_TRUE(verifyPayload(buff.data(), m_payload_sz, m_init_seqno));
323+
EXPECT_EQ(readMessage(buff.data(), buff.size()), (int) m_payload_sz);
324+
EXPECT_TRUE(verifyPayload(buff.data(), m_payload_sz, CSeqNo::incseq(m_init_seqno)));
325+
}
326+
299327
// Test dropping a message by message number and sequence number.
300328
TEST_F(CRcvBufferReadMsg, PacketDropByMsgNoSeqNo)
301329
{

0 commit comments

Comments
 (0)