From 5b27c3fa7a3c390969a3013c61b58f06fb7c0c41 Mon Sep 17 00:00:00 2001 From: Winlin Date: Mon, 15 Sep 2025 10:55:09 -0400 Subject: [PATCH] RTC2RTMP: Fix sequence number wraparound assertion crashes. v6.0.177 v7.0.89 (#4491) The issue occurred when srs_rtp_seq_distance(start, end) + 1 resulted in values <= 0 due to sequence number wraparound (e.g., when end < start). This caused assertion failures and server crashes. SrsRtcFrameBuilder::check_frame_complete(): Added validation to return false for invalid sequence ranges instead of asserting. However, it maybe cause converting RTC to RTMP stream failure, because this issue should be caused by the problem of sequence number of RTP, which means there potentially be stream problem in RTC stream. Even so, changing assert to warning logs is better, because SRS should not crash when stream is corrupt. --------- Co-authored-by: OSSRS-AI --- trunk/doc/CHANGELOG.md | 3 +++ trunk/src/app/srs_app_rtc_source.cpp | 7 +++++- trunk/src/core/srs_core_version6.hpp | 2 +- trunk/src/core/srs_core_version7.hpp | 2 +- trunk/src/utest/srs_utest_protocol3.cpp | 2 +- trunk/src/utest/srs_utest_rtc2.cpp | 29 +++++++++++++++++++++++++ 6 files changed, 41 insertions(+), 4 deletions(-) diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md index f8fc97939..a9bfb620e 100644 --- a/trunk/doc/CHANGELOG.md +++ b/trunk/doc/CHANGELOG.md @@ -7,6 +7,7 @@ The changelog for SRS. ## SRS 7.0 Changelog +* v7.0, 2025-09-15, RTC2RTMP: Fix sequence number wraparound assertion crashes. v7.0.89 (#4491) * v7.0, 2025-09-14, Merge [#4489](https://github.com/ossrs/srs/pull/4489): Improve coverage for kernel. v7.0.88 (#4489) * v7.0, 2025-09-14, Merge [#4488](https://github.com/ossrs/srs/pull/4488): AI: Add utests for kernel and protocol. v7.0.87 (#4488) * v7.0, 2025-09-13, Merge [#4486](https://github.com/ossrs/srs/pull/4486): Move some app files to kernel. v7.0.86 (#4486) @@ -102,6 +103,8 @@ The changelog for SRS. ## SRS 6.0 Changelog +* v6.0, 2025-09-15, RTC2RTMP: Fix sequence number wraparound assertion crashes. v6.0.177 (#4491) +* v6.0, 2025-09-05, RTX: Fix race condition for timer. v6.0.176 (#4470) (#4474) * v6.0, 2025-08-26, Merge [#4451](https://github.com/ossrs/srs/pull/4451): RTC: Fix null pointer crash in RTC2RTMP when start packet is missing. v6.0.175 (#4451) * v6.0, 2025-08-16, Merge [#4441](https://github.com/ossrs/srs/pull/4441): fix err memory leak in rtc to rtmp bridge. v6.0.174 (#4441) * v6.0, 2025-08-14, Merge [#4161](https://github.com/ossrs/srs/pull/4161): fix hls & dash segments cleanup. v6.0.173 (#4161) diff --git a/trunk/src/app/srs_app_rtc_source.cpp b/trunk/src/app/srs_app_rtc_source.cpp index 5b252c9cb..19788ff53 100644 --- a/trunk/src/app/srs_app_rtc_source.cpp +++ b/trunk/src/app/srs_app_rtc_source.cpp @@ -1498,7 +1498,12 @@ int32_t SrsRtcFrameBuilderVideoPacketCache::find_next_lost_sn(uint16_t current_s bool SrsRtcFrameBuilderVideoPacketCache::check_frame_complete(const uint16_t start, const uint16_t end) { int16_t cnt = srs_rtp_seq_distance(start, end) + 1; - srs_assert(cnt >= 1); + + // If the sequence range is invalid (end before start), return false + if (cnt <= 0) { + srs_warn("invalid sequence range start=%u, end=%u, cnt=%d", start, end, cnt); + return false; + } uint16_t nn_fu_start = 0; uint16_t nn_fu_end = 0; diff --git a/trunk/src/core/srs_core_version6.hpp b/trunk/src/core/srs_core_version6.hpp index 37016a7b0..a47cbcd67 100644 --- a/trunk/src/core/srs_core_version6.hpp +++ b/trunk/src/core/srs_core_version6.hpp @@ -9,6 +9,6 @@ #define VERSION_MAJOR 6 #define VERSION_MINOR 0 -#define VERSION_REVISION 175 +#define VERSION_REVISION 177 #endif diff --git a/trunk/src/core/srs_core_version7.hpp b/trunk/src/core/srs_core_version7.hpp index 857708202..d2ca902e7 100644 --- a/trunk/src/core/srs_core_version7.hpp +++ b/trunk/src/core/srs_core_version7.hpp @@ -9,6 +9,6 @@ #define VERSION_MAJOR 7 #define VERSION_MINOR 0 -#define VERSION_REVISION 88 +#define VERSION_REVISION 89 #endif \ No newline at end of file diff --git a/trunk/src/utest/srs_utest_protocol3.cpp b/trunk/src/utest/srs_utest_protocol3.cpp index fd4d96aae..78cbfc1a0 100644 --- a/trunk/src/utest/srs_utest_protocol3.cpp +++ b/trunk/src/utest/srs_utest_protocol3.cpp @@ -1021,7 +1021,7 @@ VOID TEST(ProtocolRtpTest, SrsRtpVideoBuilderPackageStapA) EXPECT_EQ(ssrc, pkt->header.get_ssrc()); EXPECT_EQ(SrsFrameTypeVideo, pkt->frame_type_); EXPECT_EQ(1000 * 90, pkt->header.get_timestamp()); // timestamp * 90 - EXPECT_FALSE(pkt->header.get_marker()); // STAP-A should not have marker bit set + EXPECT_FALSE(pkt->header.get_marker()); // STAP-A should not have marker bit set // Verify STAP-A payload was created EXPECT_TRUE(pkt->payload() != NULL); diff --git a/trunk/src/utest/srs_utest_rtc2.cpp b/trunk/src/utest/srs_utest_rtc2.cpp index b1cc8553e..2d147fcce 100644 --- a/trunk/src/utest/srs_utest_rtc2.cpp +++ b/trunk/src/utest/srs_utest_rtc2.cpp @@ -1631,3 +1631,32 @@ VOID TEST(KernelRTC2Test, SrsRtcFrameBuilderPacketVideoRtmpNullPointerCrash) EXPECT_EQ(1, bridge.frame_count); } } + +VOID TEST(KernelRTC2Test, SrsRtcFrameBuilderSequenceWrapAroundFix) +{ + // Test for the sequence number wraparound assertion fix + // + // ISSUE BACKGROUND: + // The check_frame_complete() used srs_rtp_seq_distance(start, end) + 1 + // and asserted that the result >= 1. However, when sequence numbers wrap around (e.g., end < start), + // srs_rtp_seq_distance can return negative values, causing the assertion to fail and crash the server. + // + // THE CRASH: + // For example, if start=5 and end=3, then srs_rtp_seq_distance(5, 3) = (int16_t)(3 - 5) = -2 + // So cnt = -2 + 1 = -1, which fails the assertion cnt >= 1 + // + // THE FIX: + // Added validation to check if cnt <= 0 and handle it gracefully: + // - check_frame_complete() returns false for invalid ranges + + SrsRtcFrameBuilderVideoPacketCache frame_builder; + + // Test check_frame_complete with wraparound sequence numbers + // This should not crash and should return false for invalid ranges + EXPECT_FALSE(frame_builder.check_frame_complete(5, 3)); // end < start + EXPECT_FALSE(frame_builder.check_frame_complete(100, 99)); // end < start + + // Valid cases should still work + EXPECT_TRUE(frame_builder.check_frame_complete(3, 3)); // same sequence + EXPECT_TRUE(frame_builder.check_frame_complete(3, 5)); // normal case +}