diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc index ac920aed5f..1b3e6f5665 100644 --- a/talk/app/webrtc/webrtcsdp.cc +++ b/talk/app/webrtc/webrtcsdp.cc @@ -588,6 +588,19 @@ static bool CaseInsensitiveFind(std::string str1, std::string str2) { return str1.find(str2) != std::string::npos; } +template +static bool GetValueFromString(const std::string& line, + const std::string& s, + T* t, + SdpParseError* error) { + if (!talk_base::FromString(s, t)) { + std::ostringstream description; + description << "Invalid value: " << s << "."; + return ParseFailed(line, description.str(), error); + } + return true; +} + void CreateTracksFromSsrcInfos(const SsrcInfoVec& ssrc_infos, StreamParamsVec* tracks) { ASSERT(tracks != NULL); @@ -1003,11 +1016,20 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, if (!GetValue(fields[0], kAttributeCandidate, &foundation, error)) { return false; } - const int component_id = talk_base::FromString(fields[1]); + int component_id = 0; + if (!GetValueFromString(first_line, fields[1], &component_id, error)) { + return false; + } const std::string transport = fields[2]; - const uint32 priority = talk_base::FromString(fields[3]); + uint32 priority = 0; + if (!GetValueFromString(first_line, fields[3], &priority, error)) { + return false; + } const std::string connection_address = fields[4]; - const int port = talk_base::FromString(fields[5]); + int port = 0; + if (!GetValueFromString(first_line, fields[5], &port, error)) { + return false; + } SocketAddress address(connection_address, port); cricket::ProtocolType protocol; @@ -1038,8 +1060,12 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, } if (fields.size() >= (current_position + 2) && fields[current_position] == kAttributeCandidateRport) { - related_address.SetPort( - talk_base::FromString(fields[++current_position])); + int port = 0; + if (!GetValueFromString( + first_line, fields[++current_position], &port, error)) { + return false; + } + related_address.SetPort(port); ++current_position; } @@ -1055,7 +1081,9 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, // RFC 5245 // *(SP extension-att-name SP extension-att-value) if (fields[i] == kAttributeCandidateGeneration) { - generation = talk_base::FromString(fields[++i]); + if (!GetValueFromString(first_line, fields[++i], &generation, error)) { + return false; + } } else if (fields[i] == kAttributeCandidateUsername) { username = fields[++i]; } else if (fields[i] == kAttributeCandidatePassword) { @@ -1110,7 +1138,10 @@ bool ParseExtmap(const std::string& line, RtpHeaderExtension* extmap, } std::vector sub_fields; talk_base::split(value_direction, kSdpDelimiterSlash, &sub_fields); - int value = talk_base::FromString(sub_fields[0]); + int value = 0; + if (!GetValueFromString(line, sub_fields[0], &value, error)) { + return false; + } *extmap = RtpHeaderExtension(uri, value); return true; @@ -1541,7 +1572,9 @@ bool GetParameter(const std::string& name, if (found == params.end()) { return false; } - *value = talk_base::FromString(found->second); + if (!talk_base::FromString(found->second, value)) { + return false; + } return true; } @@ -2082,7 +2115,11 @@ bool ParseMediaDescription(const std::string& message, // std::vector codec_preference; for (size_t j = 3 ; j < fields.size(); ++j) { - codec_preference.push_back(talk_base::FromString(fields[j])); + int pl = 0; + if (!GetValueFromString(line, fields[j], &pl, error)) { + return false; + } + codec_preference.push_back(pl); } // Make a temporary TransportDescription based on |session_td|. @@ -2392,19 +2429,6 @@ bool ParseContent(const std::string& message, } } - if (IsLineType(line, kLineTypeSessionBandwidth)) { - std::string bandwidth; - if (HasAttribute(line, kApplicationSpecificMaximum)) { - if (!GetValue(line, kApplicationSpecificMaximum, &bandwidth, error)) { - return false; - } else { - media_desc->set_bandwidth( - talk_base::FromString(bandwidth) * 1000); - } - } - continue; - } - // RFC 4566 // b=* (zero or more bandwidth information lines) if (IsLineType(line, kLineTypeSessionBandwidth)) { @@ -2413,8 +2437,11 @@ bool ParseContent(const std::string& message, if (!GetValue(line, kApplicationSpecificMaximum, &bandwidth, error)) { return false; } else { - media_desc->set_bandwidth( - talk_base::FromString(bandwidth) * 1000); + int b = 0; + if (!GetValueFromString(line, bandwidth, &b, error)) { + return false; + } + media_desc->set_bandwidth(b * 1000); } } continue; @@ -2537,9 +2564,11 @@ bool ParseContent(const std::string& message, return false; } int buffer_latency = 0; - if (!talk_base::FromString(flag_value, &buffer_latency) || - buffer_latency < 0) { - return ParseFailed(message, "Invalid buffer latency.", error); + if (!GetValueFromString(line, flag_value, &buffer_latency, error)) { + return false; + } + if (buffer_latency < 0) { + return ParseFailed(line, "Buffer latency less than 0.", error); } media_desc->set_buffered_mode_latency(buffer_latency); } @@ -2631,7 +2660,10 @@ bool ParseSsrcAttribute(const std::string& line, SsrcInfoVec* ssrc_infos, if (!GetValue(field1, kAttributeSsrc, &ssrc_id_s, error)) { return false; } - uint32 ssrc_id = talk_base::FromString(ssrc_id_s); + uint32 ssrc_id = 0; + if (!GetValueFromString(line, ssrc_id_s, &ssrc_id, error)) { + return false; + } std::string attribute; std::string value; @@ -2708,7 +2740,10 @@ bool ParseSsrcGroupAttribute(const std::string& line, } std::vector ssrcs; for (size_t i = 1; i < fields.size(); ++i) { - uint32 ssrc = talk_base::FromString(fields[i]); + uint32 ssrc = 0; + if (!GetValueFromString(line, fields[i], &ssrc, error)) { + return false; + } ssrcs.push_back(ssrc); } ssrc_groups->push_back(SsrcGroup(semantics, ssrcs)); @@ -2731,7 +2766,10 @@ bool ParseCryptoAttribute(const std::string& line, if (!GetValue(fields[0], kAttributeCrypto, &tag_value, error)) { return false; } - int tag = talk_base::FromString(tag_value); + int tag = 0; + if (!GetValueFromString(line, tag_value, &tag, error)) { + return false; + } const std::string crypto_suite = fields[1]; const std::string key_params = fields[2]; std::string session_params; @@ -2795,7 +2833,10 @@ bool ParseRtpmapAttribute(const std::string& line, if (!GetValue(fields[0], kAttributeRtpmap, &payload_type_value, error)) { return false; } - const int payload_type = talk_base::FromString(payload_type_value); + int payload_type = 0; + if (!GetValueFromString(line, payload_type_value, &payload_type, error)) { + return false; + } // Set the preference order depending on the order of the pl type in the // of the m-line. @@ -2819,7 +2860,10 @@ bool ParseRtpmapAttribute(const std::string& line, error); } const std::string encoding_name = codec_params[0]; - const int clock_rate = talk_base::FromString(codec_params[1]); + int clock_rate = 0; + if (!GetValueFromString(line, codec_params[1], &clock_rate, error)) { + return false; + } if (media_type == cricket::MEDIA_TYPE_VIDEO) { VideoContentDescription* video_desc = static_cast(media_desc); @@ -2838,7 +2882,9 @@ bool ParseRtpmapAttribute(const std::string& line, // additional parameters are needed. int channels = 1; if (codec_params.size() == 3) { - channels = talk_base::FromString(codec_params[2]); + if (!GetValueFromString(line, codec_params[2], &channels, error)) { + return false; + } } int bitrate = 0; // The default behavior for ISAC (bitrate == 0) in webrtcvoiceengine.cc @@ -2925,7 +2971,10 @@ bool ParseFmtpAttributes(const std::string& line, const MediaType media_type, codec_params[name] = value; } - int int_payload_type = talk_base::FromString(payload_type); + int int_payload_type = 0; + if (!GetValueFromString(line, payload_type, &int_payload_type, error)) { + return false; + } if (media_type == cricket::MEDIA_TYPE_AUDIO) { UpdateCodec( media_desc, int_payload_type, codec_params); @@ -2953,8 +3002,12 @@ bool ParseRtcpFbAttribute(const std::string& line, const MediaType media_type, error)) { return false; } - int payload_type = (payload_type_string == "*") ? - kWildcardPayloadType : talk_base::FromString(payload_type_string); + int payload_type = kWildcardPayloadType; + if (payload_type_string != "*") { + if (!GetValueFromString(line, payload_type_string, &payload_type, error)) { + return false; + } + } std::string id = rtcp_fb_fields[1]; std::string param = ""; for (std::vector::iterator iter = rtcp_fb_fields.begin() + 2; diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc index 76765aa4be..aec144745f 100644 --- a/talk/app/webrtc/webrtcsdp_unittest.cc +++ b/talk/app/webrtc/webrtcsdp_unittest.cc @@ -401,14 +401,31 @@ static void Replace(const std::string& line, newlines.c_str(), newlines.length(), message); } -static void ReplaceAndTryToParse(const char* search, const char* replace) { +// Expect fail to parase |bad_sdp| and expect |bad_part| be part of the error +// message. +static void ExpectParseFailure(const std::string& bad_sdp, + const std::string& bad_part) { JsepSessionDescription desc(kDummyString); - std::string sdp = kSdpFullString; - Replace(search, replace, &sdp); SdpParseError error; - bool ret = webrtc::SdpDeserialize(sdp, &desc, &error); + bool ret = webrtc::SdpDeserialize(bad_sdp, &desc, &error); EXPECT_FALSE(ret); - EXPECT_NE(std::string::npos, error.line.find(replace)); + EXPECT_NE(std::string::npos, error.line.find(bad_part.c_str())); +} + +// Expect fail to parse kSdpFullString if replace |good_part| with |bad_part|. +static void ExpectParseFailure(const char* good_part, const char* bad_part) { + std::string bad_sdp = kSdpFullString; + Replace(good_part, bad_part, &bad_sdp); + ExpectParseFailure(bad_sdp, bad_part); +} + +// Expect fail to parse kSdpFullString if add |newlines| after |injectpoint|. +static void ExpectParseFailureWithNewLines(const std::string& injectpoint, + const std::string& newlines, + const std::string& bad_part) { + std::string bad_sdp = kSdpFullString; + InjectAfter(injectpoint, newlines, &bad_sdp); + ExpectParseFailure(bad_sdp, bad_part); } static void ReplaceDirection(cricket::MediaContentDirection direction, @@ -2017,28 +2034,67 @@ TEST_F(WebRtcSdpTest, DeserializeBrokenSdp) { "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B"; // Broken session description - ReplaceAndTryToParse("v=", kSdpDestroyer); - ReplaceAndTryToParse("o=", kSdpDestroyer); - ReplaceAndTryToParse("s=-", kSdpDestroyer); + ExpectParseFailure("v=", kSdpDestroyer); + ExpectParseFailure("o=", kSdpDestroyer); + ExpectParseFailure("s=-", kSdpDestroyer); // Broken time description - ReplaceAndTryToParse("t=", kSdpDestroyer); + ExpectParseFailure("t=", kSdpDestroyer); // Broken media description - ReplaceAndTryToParse("m=audio", "c=IN IP4 74.125.224.39"); - ReplaceAndTryToParse("m=video", kSdpDestroyer); + ExpectParseFailure("m=audio", "c=IN IP4 74.125.224.39"); + ExpectParseFailure("m=video", kSdpDestroyer); // Invalid lines - ReplaceAndTryToParse("a=candidate", kSdpInvalidLine1); - ReplaceAndTryToParse("a=candidate", kSdpInvalidLine2); - ReplaceAndTryToParse("a=candidate", kSdpInvalidLine3); + ExpectParseFailure("a=candidate", kSdpInvalidLine1); + ExpectParseFailure("a=candidate", kSdpInvalidLine2); + ExpectParseFailure("a=candidate", kSdpInvalidLine3); // Bogus fingerprint replacing a=sendrev. We selected this attribute // because it's orthogonal to what we are replacing and hence // safe. - ReplaceAndTryToParse("a=sendrecv", kSdpInvalidLine4); - ReplaceAndTryToParse("a=sendrecv", kSdpInvalidLine5); - ReplaceAndTryToParse("a=sendrecv", kSdpInvalidLine6); - ReplaceAndTryToParse("a=sendrecv", kSdpInvalidLine7); + ExpectParseFailure("a=sendrecv", kSdpInvalidLine4); + ExpectParseFailure("a=sendrecv", kSdpInvalidLine5); + ExpectParseFailure("a=sendrecv", kSdpInvalidLine6); + ExpectParseFailure("a=sendrecv", kSdpInvalidLine7); +} + +TEST_F(WebRtcSdpTest, DeserializeSdpWithInvalidAttributeValue) { + // ssrc + ExpectParseFailure("a=ssrc:1", "a=ssrc:badvalue"); + ExpectParseFailure("a=ssrc-group:FEC 5 6", "a=ssrc-group:FEC badvalue 6"); + // crypto + ExpectParseFailure("a=crypto:1 ", "a=crypto:badvalue "); + // rtpmap + ExpectParseFailure("a=rtpmap:111 ", "a=rtpmap:badvalue "); + ExpectParseFailure("opus/48000/2", "opus/badvalue/2"); + ExpectParseFailure("opus/48000/2", "opus/48000/badvalue"); + // candidate + ExpectParseFailure("1 udp 2130706432", "badvalue udp 2130706432"); + ExpectParseFailure("1 udp 2130706432", "1 udp badvalue"); + ExpectParseFailure("192.168.1.5 1234", "192.168.1.5 badvalue"); + ExpectParseFailure("rport 2346", "rport badvalue"); + ExpectParseFailure("rport 2346 generation 2", + "rport 2346 generation badvalue"); + // m line + ExpectParseFailure("m=audio 2345 RTP/SAVPF 111 103 104", + "m=audio 2345 RTP/SAVPF 111 badvalue 104"); + + // bandwidth + ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", + "b=AS:badvalue\r\n", + "b=AS:badvalue"); + // rtcp-fb + ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", + "a=rtcp-fb:badvalue nack\r\n", + "a=rtcp-fb:badvalue nack"); + // extmap + ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", + "a=extmap:badvalue http://example.com\r\n", + "a=extmap:badvalue http://example.com"); + // x-google-buffer-latency + ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", + "a=x-google-buffer-latency:badvalue\r\n", + "a=x-google-buffer-latency:badvalue"); } TEST_F(WebRtcSdpTest, DeserializeSdpWithReorderedPltypes) {