admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ignore ufrag/password in "a=candidate" lines in SDP.

Browse Source 
These attributes should be parsed in candidate trickling, but when
parsing a full session description, only "a=ice-ufrag"/"a=ice-pwd"
should be used to communicate the ufrag/password.

BUG=chromium:681286

Review-Url: https://codereview.webrtc.org/2639183002
Cr-Commit-Position: refs/heads/master@{#16194}
This commit is contained in:
deadbeef 2017-01-20 12:43:58 -08:00 committed by Commit bot
parent 0fc04b74c5
commit 7bcdb69957
2 changed files with 38 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
webrtc/api/webrtcsdp.cc
View File

@ -2676,6 +2676,12 @@ bool ParseContent(const std::string& message,
if (!ParseCandidate(line, &candidate, error, false)) {
return false;
}
// ParseCandidate will parse non-standard ufrag and password attributes,
// since it's used for candidate trickling, but we only want to process
// the "a=ice-ufrag"/"a=ice-pwd" values in a session description, so
// strip them off at this point.
candidate.set_username(std::string());
candidate.set_password(std::string());
candidates_orig.push_back(candidate);
} else if (HasAttribute(line, kAttributeIceUfrag)) {
if (!GetValue(line, kAttributeIceUfrag, &transport->ice_ufrag, error)) {

38
webrtc/api/webrtcsdp_unittest.cc
View File

@ -3311,11 +3311,9 @@ TEST_F(WebRtcSdpTest, SerializeUnifiedPlanSessionDescription) {
// 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[] =
static const char kSdpWithSctpPortInVideoDescription[] =
"v=0\r\n"
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
@ -3331,11 +3329,9 @@ TEST_F(WebRtcSdpTest, DeserializeSctpPortInVideoDescription) {
// Regression test for integer overflow bug:
// https://bugs.chromium.org/p/chromium/issues/detail?id=648071
TEST_F(WebRtcSdpTest, DeserializeLargeBandwidthLimit) {
JsepSessionDescription jdesc_output(kDummyString);
// Bandwidth attribute is the max signed 32-bit int, which will get
// multiplied by 1000 and cause int overflow if not careful.
const char kSdpWithLargeBandwidth[] =
static const char kSdpWithLargeBandwidth[] =
"v=0\r\n"
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
@ -3346,3 +3342,33 @@ TEST_F(WebRtcSdpTest, DeserializeLargeBandwidthLimit) {
ExpectParseFailure(std::string(kSdpWithLargeBandwidth), "foo=fail");
}
// Test that "ufrag"/"pwd" in the candidate line itself are ignored, and only
// the "a=ice-ufrag"/"a=ice-pwd" attributes are used.
// Regression test for:
// https://bugs.chromium.org/p/chromium/issues/detail?id=681286
TEST_F(WebRtcSdpTest, IceCredentialsInCandidateStringIgnored) {
// Important piece is "ufrag foo pwd bar".
static const char kSdpWithIceCredentialsInCandidateString[] =
"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"
"c=IN IP4 0.0.0.0\r\n"
"a=rtcp:9 IN IP4 0.0.0.0\r\n"
"a=ice-ufrag:ufrag_voice\r\na=ice-pwd:pwd_voice\r\n"
"a=rtpmap:111 opus/48000/2\r\n"
"a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 1234 typ host "
"generation 2 ufrag foo pwd bar\r\n";
JsepSessionDescription jdesc_output(kDummyString);
EXPECT_TRUE(
SdpDeserialize(kSdpWithIceCredentialsInCandidateString, &jdesc_output));
const IceCandidateCollection* candidates = jdesc_output.candidates(0);
ASSERT_NE(nullptr, candidates);
ASSERT_EQ(1, candidates->count());
cricket::Candidate c = candidates->at(0)->candidate();
EXPECT_EQ("ufrag_voice", c.username());
EXPECT_EQ("pwd_voice", c.password());
}