sdp: cross-check media type and protocol earlier

catching some unsupported configurations earlier.

BUG=None

Change-Id: I9f366929816fe15031837a3218086aa5a13d787a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/197813
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
Cr-Commit-Position: refs/heads/master@{#33084}
This commit is contained in:
Philipp Hancke 2021-01-13 10:40:06 +01:00 committed by Commit Bot
parent 2aad81259e
commit b70c9531ee
2 changed files with 26 additions and 32 deletions

View File

@ -2660,6 +2660,10 @@ bool ParseMediaDescription(
bool bundle_only = false;
int section_msid_signaling = 0;
const std::string& media_type = fields[0];
if ((media_type == kMediaTypeVideo || media_type == kMediaTypeAudio) &&
!cricket::IsRtpProtocol(protocol)) {
return ParseFailed(line, "Unsupported protocol for media type", error);
}
if (media_type == kMediaTypeVideo) {
content = ParseContentDescription<VideoContentDescription>(
message, cricket::MEDIA_TYPE_VIDEO, mline_index, protocol,
@ -2696,7 +2700,7 @@ bool ParseMediaDescription(
}
data_desc->set_protocol(protocol);
content = std::move(data_desc);
} else {
} else if (cricket::IsRtpProtocol(protocol)) {
// RTP
std::unique_ptr<RtpDataContentDescription> data_desc =
ParseContentDescription<RtpDataContentDescription>(
@ -2704,6 +2708,8 @@ bool ParseMediaDescription(
payload_types, pos, &content_name, &bundle_only,
&section_msid_signaling, &transport, candidates, error);
content = std::move(data_desc);
} else {
return ParseFailed(line, "Unsupported protocol for media type", error);
}
} else {
RTC_LOG(LS_WARNING) << "Unsupported media type: " << line;
@ -3128,11 +3134,6 @@ bool ParseContent(const std::string& message,
// SCTP specific attributes
//
if (HasAttribute(line, kAttributeSctpPort)) {
if (media_type != cricket::MEDIA_TYPE_DATA) {
return ParseFailed(
line, "sctp-port attribute found in non-data media description.",
error);
}
if (media_desc->as_sctp()->use_sctpmap()) {
return ParseFailed(
line, "sctp-port attribute can't be used with sctpmap.", error);
@ -3143,12 +3144,6 @@ bool ParseContent(const std::string& message,
}
media_desc->as_sctp()->set_port(sctp_port);
} else if (HasAttribute(line, kAttributeMaxMessageSize)) {
if (media_type != cricket::MEDIA_TYPE_DATA) {
return ParseFailed(
line,
"max-message-size attribute found in non-data media description.",
error);
}
int max_message_size;
if (!ParseSctpMaxMessageSize(line, &max_message_size, error)) {
return false;

View File

@ -944,8 +944,8 @@ static void Replace(const std::string& line,
absl::StrReplaceAll({{line, newlines}}, message);
}
// Expect fail to parase |bad_sdp| and expect |bad_part| be part of the error
// message.
// Expect a parse failure on the line containing |bad_part| when attempting to
// parse |bad_sdp|.
static void ExpectParseFailure(const std::string& bad_sdp,
const std::string& bad_part) {
JsepSessionDescription desc(kDummyType);
@ -4058,24 +4058,6 @@ TEST_F(WebRtcSdpTest, SerializeBothMediaSectionAndSsrcAttributeMsid) {
EXPECT_NE(std::string::npos, sdp.find(kSsrcAttributeMsidLine));
}
// Regression test for heap overflow bug:
// https://bugs.chromium.org/p/chromium/issues/detail?id=647916
TEST_F(WebRtcSdpTest, DeserializeSctpPortInVideoDescription) {
// The issue occurs when the sctp-port attribute is found in a video
// description. The actual heap overflow occurs when parsing the fmtp line.
static const char kSdpWithSctpPortInVideoDescription[] =
"v=0\r\n"
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
"m=video 9 UDP/DTLS/SCTP 120\r\n"
"a=sctp-port 5000\r\n"
"a=fmtp:108 foo=10\r\n";
ExpectParseFailure(std::string(kSdpWithSctpPortInVideoDescription),
"sctp-port");
}
// Regression test for integer overflow bug:
// https://bugs.chromium.org/p/chromium/issues/detail?id=648071
TEST_F(WebRtcSdpTest, DeserializeLargeBandwidthLimit) {
@ -4761,3 +4743,20 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithUnsupportedMediaType) {
EXPECT_EQ(jdesc_output.description()->contents()[0].name, "bogusmid");
EXPECT_EQ(jdesc_output.description()->contents()[1].name, "somethingmid");
}
TEST_F(WebRtcSdpTest, MediaTypeProtocolMismatch) {
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";
ExpectParseFailure(std::string(sdp + "m=audio 9 UDP/DTLS/SCTP 120\r\n"),
"m=audio");
ExpectParseFailure(std::string(sdp + "m=video 9 UDP/DTLS/SCTP 120\r\n"),
"m=video");
ExpectParseFailure(std::string(sdp + "m=video 9 SOMETHING 120\r\n"),
"m=video");
ExpectParseFailure(std::string(sdp + "m=application 9 SOMETHING 120\r\n"),
"m=application");
}