[BUG] Fixed OOB read in LOSSREPORT range parsing (#3324)

Author: mszatmary-netflix

srtcore/core.cpp

@@ -9070,6 +9070,14 @@ void srt::CUDT::processCtrlLossReport(const CPacket& ctrlpkt)
             // IF the loss is a range <LO, HI>
             if (IsSet(losslist[i], LOSSDATA_SEQNO_RANGE_FIRST))
             {
+                // The range-first marker means the next cell holds HI. Reject if it isn't present
+                // in the body — otherwise losslist[i+1] reads past the wire payload.
+                if (i + 1 >= n)
+                {
+                    LOGC(inlog.Warn, log << CONID() << "rcv LOSSREPORT: trailing range-first word with no HI - DISCARDING");
+                    secure = false;
+                    break;
+                }
                 // Then it's this is a <LO, HI> specification with HI in a consecutive cell.
                 const int32_t losslist_lo = SEQNO_VALUE::unwrap(losslist[i]);
                 const int32_t losslist_hi = losslist[i + 1];

test/test_control_packets.cpp

@@ -17,8 +17,10 @@ namespace srt {
         CUDT* core;
 
         void processCtrlDropReq(const CPacket& pkt) { core->processCtrlDropReq(pkt); }
+        void processCtrlLossReport(const CPacket& pkt) { core->processCtrlLossReport(pkt); }
         int32_t rcvCurrSeqNo() const { return core->m_iRcvCurrSeqNo; }
         void setRcvCurrSeqNo(int32_t v) { core->m_iRcvCurrSeqNo = v; }
+        bool isBroken() const { return core->m_bBroken; }
     };
 }
 
@@ -55,3 +57,40 @@ TEST(ControlPackets, DropReqRejectsShortPayload)
     pkt.deallocate();
     srt_close(sid1);
 }
+
+// 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
+// adjacent heap). The handler should mark the connection broken via the
+// `secure = false` path.
+TEST(ControlPackets, LossReportRejectsTrailingRangeFirst)
+{
+    srt::TestInit srtinit;
+
+    CUDTSocket* s = NULL;
+    SRTSOCKET sid = CUDT::uglobal().newSocket(&s);
+    ASSERT_NE(sid, SRT_INVALID_SOCK);
+
+    TestMockControlPackets m;
+    m.core = &s->core();
+    ASSERT_FALSE(m.isBroken()) << "fresh socket should not be marked broken";
+
+    // Single 4-byte payload, high bit set => LOSSDATA_SEQNO_RANGE_FIRST.
+    // Without the guard, the handler would dereference losslist[1] (the
+    // missing HI cell), reading 4 bytes past the packet payload.
+    CPacket pkt;
+    pkt.allocate(64);
+    int32_t* data = (int32_t*) pkt.m_pcData;
+    data[0] = SEQNO_VALUE::wrap(100) | LOSSDATA_SEQNO_RANGE_FIRST;
+    pkt.setLength(sizeof(int32_t));
+    pkt.setControl(UMSG_LOSSREPORT);
+
+    m.processCtrlLossReport(pkt);
+
+    EXPECT_TRUE(m.isBroken())
+        << "LOSSREPORT with trailing range-first marker must take the "
+           "secure=false bail path and mark the connection broken";
+
+    pkt.deallocate();
+    srt_close(sid);
+}