From 7336ad162f560e5b2ea96afba02d5f0bd57d6139 Mon Sep 17 00:00:00 2001 From: Matthew Szatmary Date: Tue, 19 May 2026 07:44:47 -0700 Subject: [PATCH] [BUG] Reject DROPREQ with reversed seqno range processCtrlDropReq read dropdata[0] (lo) and dropdata[1] (hi) from the wire without checking seqcmp(lo, hi). When lo > hi, the call to CRcvBuffer::dropMessage(lo, hi, ...) computes offset_a = seqoff(start, lo) > offset_b = seqoff(start, hi), and the loop walks the circular buffer via incPos() until it wraps around to end_pos -- wiping nearly every entry in the receive buffer. A single malicious DROPREQ packet is a clean DoS primitive. The analogous LOSSREPORT handler (processCtrlLossReport) already rejects reversed ranges. Mirror that defense here: validate seqcmp(dropdata[0], dropdata[1]) <= 0 before doing any work. Add ControlPackets.DropReqRejectsReversedRange covering the new check via the existing TestMockControlPackets friend wrapper. Co-Authored-By: Claude Opus 4.7 (1M context) --- srtcore/core.cpp | 12 ++++++++++++ test/test_control_packets.cpp | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index e6087fd68..ef2e77b96 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -9335,6 +9335,18 @@ void srt::CUDT::processCtrlDropReq(const CPacket& ctrlpkt) const int32_t* dropdata = (const int32_t*) ctrlpkt.m_pcData; + // The wire format carries a (lo, hi) seqno range. Reject reversed + // ranges: dropMessage walks a circular buffer from offset(lo) to + // offset(hi)+1 via incPos(); when seqcmp(lo, hi) > 0 the loop wraps + // and clears nearly the entire receive buffer (DoS primitive). The + // analogous LOSSREPORT path already rejects reversed ranges. + if (CSeqNo::seqcmp(dropdata[0], dropdata[1]) > 0) + { + LOGC(inlog.Warn, log << CONID() << "rcv DROPREQ rng %" + << dropdata[0] << " - %" << dropdata[1] << " - REVERSED RANGE, DISCARDING"); + return; + } + { CUniqueSync rcvtscc (m_RecvLock, m_RcvTsbPdCond); // With both TLPktDrop and TsbPd enabled, a message always consists only of one packet. diff --git a/test/test_control_packets.cpp b/test/test_control_packets.cpp index 6b7f322c5..88e6cbeae 100644 --- a/test/test_control_packets.cpp +++ b/test/test_control_packets.cpp @@ -55,3 +55,39 @@ TEST(ControlPackets, DropReqRejectsShortPayload) pkt.deallocate(); srt_close(sid1); } + +// processCtrlDropReq must reject DROPREQs whose (lo, hi) seqno range is +// reversed. Otherwise CRcvBuffer::dropMessage walks the circular buffer from +// offset(lo) past offset(hi)+1 via incPos() and wipes nearly every entry -- +// a DoS primitive triggerable by a single malicious DROPREQ. +TEST(ControlPackets, DropReqRejectsReversedRange) +{ + srt::TestInit srtinit; + + CUDTSocket* s = NULL; + SRTSOCKET sid = CUDT::uglobal().newSocket(&s); + ASSERT_NE(sid, SRT_INVALID_SOCK); + + TestMockControlPackets m; + m.core = &s->core(); + + const int32_t sentinel = 1000; + m.setRcvCurrSeqNo(sentinel); + + CPacket pkt; + pkt.allocate(8); + int32_t* data = (int32_t*) pkt.m_pcData; + data[0] = 2000; // lo + data[1] = 1500; // hi (seqcmp(lo, hi) > 0) + pkt.setLength(8); + pkt.setControl(UMSG_DROPREQ); + + // With the guard, this returns before touching m_pRcvBuffer (NULL on + // an unconnected socket -- would crash if the guard were missing). + m.processCtrlDropReq(pkt); + + EXPECT_EQ(m.rcvCurrSeqNo(), sentinel); + + pkt.deallocate(); + srt_close(sid); +}