From 5e795a4d4e612ceefb0a0c3dd7baa16787863c05 Mon Sep 17 00:00:00 2001 From: ubeddulla khan Date: Sat, 4 Jul 2026 14:44:28 +0530 Subject: [PATCH] fix out-of-bounds read parsing AVC SPS in ParseSPS --- src/brpc/rtmp.cpp | 2 +- test/brpc_rtmp_unittest.cpp | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/brpc/rtmp.cpp b/src/brpc/rtmp.cpp index 4913881cb1..af8e2a7281 100644 --- a/src/brpc/rtmp.cpp +++ b/src/brpc/rtmp.cpp @@ -612,7 +612,7 @@ butil::Status AVCDecoderConfigurationRecord::Create(const void* data, size_t len return butil::Status(EINVAL, "Not enough data to decode SPS"); } if (sps_length > 0) { - butil::Status st = ParseSPS(buf.data() + 2, sps_length); + butil::Status st = ParseSPS(buf.substr(2, sps_length), sps_length); if (!st.ok()) { return st; } diff --git a/test/brpc_rtmp_unittest.cpp b/test/brpc_rtmp_unittest.cpp index 6834036a3a..8b8a59e71a 100644 --- a/test/brpc_rtmp_unittest.cpp +++ b/test/brpc_rtmp_unittest.cpp @@ -729,6 +729,25 @@ TEST(RtmpTest, amf_rejects_deep_nested_ecma_arrays) { EXPECT_TRUE(brpc::ReadAMFObject(&valid_obj, &istream2)); } +// Create() copies the record into a non-NUL-terminated buffer, so the SPS must +// be parsed with an explicitly-sized view. A crafted sequence header whose SPS +// body has no zero byte used to make ParseSPS run strlen off the end of that +// buffer (an out-of-bounds read, caught here under ASan). +TEST(RtmpTest, avc_seq_header_sps_without_zero_byte) { + const uint16_t sps_length = 70; // keep the record above the small-array cap + butil::IOBuf buf; + const char head[6] = { 0x01, 0x64, 0x00, 0x28, (char)0xff, (char)0xe1 }; + buf.append(head, sizeof(head)); // version/profile/level, lengthSizeMinus1=3, numSPS=1 + const char len_be[2] = { (char)(sps_length >> 8), (char)(sps_length & 0xff) }; + buf.append(len_be, sizeof(len_be)); + std::string sps(sps_length, (char)0x67); // NAL header 0x67 then non-zero filler + buf.append(sps); + + brpc::AVCDecoderConfigurationRecord avc; + // Only requirement: the call must not read past the copied record. + avc.Create(buf); +} + TEST(RtmpTest, successfully_play_streams) { PlayingDummyService rtmp_service; brpc::Server server;