From 3c8054495884326956bfbdfd467ccf34da504b6f 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 (#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 | 1 + trunk/src/app/srs_app_rtc_source.cpp | 7 ++++++- trunk/src/core/srs_core_version6.hpp | 2 +- trunk/src/utest/srs_utest_rtc.cpp | 30 ++++++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md index 0b21f0f32..6122b2121 100644 --- a/trunk/doc/CHANGELOG.md +++ b/trunk/doc/CHANGELOG.md @@ -7,6 +7,7 @@ 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) diff --git a/trunk/src/app/srs_app_rtc_source.cpp b/trunk/src/app/srs_app_rtc_source.cpp index 6a2eb4880..229ca049f 100644 --- a/trunk/src/app/srs_app_rtc_source.cpp +++ b/trunk/src/app/srs_app_rtc_source.cpp @@ -1957,7 +1957,12 @@ void SrsRtcFrameBuilder::clear_cached_video() bool SrsRtcFrameBuilder::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 fu_s_c = 0; uint16_t fu_e_c = 0; diff --git a/trunk/src/core/srs_core_version6.hpp b/trunk/src/core/srs_core_version6.hpp index 9ff033141..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 176 +#define VERSION_REVISION 177 #endif diff --git a/trunk/src/utest/srs_utest_rtc.cpp b/trunk/src/utest/srs_utest_rtc.cpp index 4b4056b0b..0e53b0054 100644 --- a/trunk/src/utest/srs_utest_rtc.cpp +++ b/trunk/src/utest/srs_utest_rtc.cpp @@ -1486,3 +1486,33 @@ 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 + + MockStreamBridge bridge; + SrsRtcFrameBuilder frame_builder(&bridge); + + // 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 +}