Reject SDP with duplicate msid lines

This is an obscure error that was found by a fuzzer.

Bug: webrtc:15845
Change-Id: I3509fa264a3af6f0f5e8e6b75a8b7dcd8fb0da1a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339681
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41743}
This commit is contained in:
Harald Alvestrand 2024-02-14 18:08:47 +00:00 committed by WebRTC LUCI CQ
parent 611f21d0d4
commit 6a8236617d
2 changed files with 42 additions and 0 deletions

View File

@ -2341,6 +2341,7 @@ static bool ParseMsidAttribute(absl::string_view line,
// Note that JSEP stipulates not sending msid-appdata so
// a=msid:<stream id> <track id>
// is supported for backward compability reasons only.
// RFC 8830 section 2 states that duplicate a=msid:stream track is illegal.
std::vector<std::string> fields;
size_t num_fields = rtc::tokenize(line.substr(kLinePrefixLength),
kSdpDelimiterSpaceChar, &fields);
@ -2670,6 +2671,21 @@ static std::unique_ptr<MediaContentDescription> ParseContentDescription(
return media_desc;
}
bool HasDuplicateMsidLines(cricket::SessionDescription* desc) {
std::set<std::pair<std::string, std::string>> seen_msids;
for (const cricket::ContentInfo& content : desc->contents()) {
for (const cricket::StreamParams& stream :
content.media_description()->streams()) {
auto msid = std::pair(stream.first_stream_id(), stream.id);
if (seen_msids.find(msid) != seen_msids.end()) {
return true;
}
seen_msids.insert(std::move(msid));
}
}
return false;
}
bool ParseMediaDescription(
absl::string_view message,
const TransportDescription& session_td,
@ -2852,6 +2868,11 @@ bool ParseMediaDescription(
// Create TransportInfo with the media level "ice-pwd" and "ice-ufrag".
desc->AddTransportInfo(TransportInfo(content_name, transport));
}
// Apply whole-description sanity checks
if (HasDuplicateMsidLines(desc)) {
ParseFailed(message, *pos, "Duplicate a=msid lines detected", error);
return false;
}
desc->set_msid_signaling(msid_signaling);

View File

@ -4271,6 +4271,27 @@ TEST_F(WebRtcSdpTest, DeserializeMsidAttributeWithMissingStreamId) {
EXPECT_FALSE(SdpDeserialize(sdp, &jdesc_output));
}
TEST_F(WebRtcSdpTest, DeserializeMsidAttributeWithDuplicateStreamIdAndTrackId) {
std::string sdp =
"v=0\r\n"
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
"m=audio 9 RTP/SAVPF 111\r\n"
"a=mid:0\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=rtpmap:111 opus/48000/2\r\n"
"a=msid:stream_id track_id\r\n"
"m=audio 9 RTP/SAVPF 111\r\n"
"a=mid:1\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=rtpmap:111 opus/48000/2\r\n"
"a=msid:stream_id track_id\r\n";
JsepSessionDescription jdesc_output(kDummyType);
EXPECT_FALSE(SdpDeserialize(sdp, &jdesc_output));
}
// Tests that if both session-level address and media-level address exist, use
// the media-level address.
TEST_F(WebRtcSdpTest, ParseConnectionData) {