diff --git a/srtcore/core.cpp b/srtcore/core.cpp index e4577a27d..7aa6e4db3 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -9343,6 +9343,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 98ad37831..f8dd4dc7c 100644 --- a/test/test_control_packets.cpp +++ b/test/test_control_packets.cpp @@ -58,6 +58,42 @@ TEST(ControlPackets, DropReqRejectsShortPayload) 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); +} + // processCtrlLossReport must reject a LOSSREPORT whose final cell carries // the LOSSDATA_SEQNO_RANGE_FIRST marker but has no HI cell behind it; // otherwise losslist[i+1] reads past the wire payload (4-byte OOB read of