From 7e146cb97e27644691a8017fe252dfc184c03808 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Wed, 28 Sep 2016 10:04:34 -0700 Subject: [PATCH] Fixing heap read overflow when "sctp-port" is in a video description. This added an SCTP codec, which is later re-interpreted as a video codec. We shouldn't be adding codecs that don't match the type of the media description. BUG=chromium:648062 Review-Url: https://codereview.webrtc.org/2354723002 Cr-Commit-Position: refs/heads/master@{#14421} --- webrtc/api/webrtcsdp.cc | 5 +++++ webrtc/api/webrtcsdp_unittest.cc | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc index 7238131cfe..4df4ef8566 100644 --- a/webrtc/api/webrtcsdp.cc +++ b/webrtc/api/webrtcsdp.cc @@ -2651,6 +2651,11 @@ bool ParseContent(const std::string& message, return false; } } else if (IsDtlsSctp(protocol) && HasAttribute(line, kAttributeSctpPort)) { + if (media_type != cricket::MEDIA_TYPE_DATA) { + return ParseFailed( + line, "sctp-port attribute found in non-data media description.", + error); + } int sctp_port; if (!ParseSctpPort(line, &sctp_port, error)) { return false; diff --git a/webrtc/api/webrtcsdp_unittest.cc b/webrtc/api/webrtcsdp_unittest.cc index 10ff0aef25..16722689a2 100644 --- a/webrtc/api/webrtcsdp_unittest.cc +++ b/webrtc/api/webrtcsdp_unittest.cc @@ -3183,3 +3183,23 @@ TEST_F(WebRtcSdpTest, SerializeUnifiedPlanSessionDescription) { MakeUnifiedPlanDescription(); TestSerialize(jdesc_, true); } + +// Regression test for heap overflow bug: +// https://bugs.chromium.org/p/chromium/issues/detail?id=647916 +TEST_F(WebRtcSdpTest, DeserializeSctpPortInVideoDescription) { + JsepSessionDescription jdesc_output(kDummyString); + + // The issue occurs when the sctp-port attribute is found in a video + // description. The actual heap overflow occurs when parsing the fmtp line. + 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"); +}