commit 99bcf60a41206a3c1c9b9d7608e8eac5f32ddbdd
author Harald Alvestrand <hta@webrtc.org> Wed Mar 03 07:44:39 2021
committer Commit Bot <commit-bot@chromium.org> Wed Mar 03 08:55:39 2021
tree 65db3ac3f2c1e5940aa5b563cfe05f68d3160f4b
parent c67b77eee4b08c05638a219723a9141a65015da4

Check MID for illegal token characters.

Bug: webrtc:12516
Change-Id: I311dc984aa1dc8784d3ba3394676337b35cc92d9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/209360
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33370}

pc/webrtc_sdp.cc

diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index c4ebf59..26eb4f3 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -107,6 +107,15 @@
 // the form:
 // <type>=<value>
 // where <type> MUST be exactly one case-significant character.
+
+// Legal characters in a <token> value (RFC 4566 section 9):
+//    token-char =          %x21 / %x23-27 / %x2A-2B / %x2D-2E / %x30-39
+//                         / %x41-5A / %x5E-7E
+static const char kLegalTokenCharacters[] =
+    "!#$%&'*+-."                          // %x21, %x23-27, %x2A-2B, %x2D-2E
+    "0123456789"                          // %x30-39
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"          // %x41-5A
+    "^_`abcdefghijklmnopqrstuvwxyz{|}~";  // %x5E-7E
 static const int kLinePrefixLength = 2;  // Length of <type>=
 static const char kLineTypeVersion = 'v';
 static const char kLineTypeOrigin = 'o';
@@ -619,6 +628,22 @@
   return true;
 }
 
+// Get a single [token] from <attribute>:<token>
+static bool GetSingleTokenValue(const std::string& message,
+                                const std::string& attribute,
+                                std::string* value,
+                                SdpParseError* error) {
+  if (!GetValue(message, attribute, value, error)) {
+    return false;
+  }
+  if (strspn(value->c_str(), kLegalTokenCharacters) != value->size()) {
+    rtc::StringBuilder description;
+    description << "Illegal character found in the value of " << attribute;
+    return ParseFailed(message, description.str(), error);
+  }
+  return true;
+}
+
 static bool CaseInsensitiveFind(std::string str1, std::string str2) {
   absl::c_transform(str1, str1.begin(), ::tolower);
   absl::c_transform(str2, str2.begin(), ::tolower);
@@ -3099,7 +3124,7 @@
       // mid-attribute      = "a=mid:" identification-tag
       // identification-tag = token
       // Use the mid identification-tag as the content name.
-      if (!GetValue(line, kAttributeMid, &mline_id, error)) {
+      if (!GetSingleTokenValue(line, kAttributeMid, &mline_id, error)) {
         return false;
       }
       *content_name = mline_id;

pc/webrtc_sdp_unittest.cc

diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 296781f..5e7c225 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -951,8 +951,9 @@
   JsepSessionDescription desc(kDummyType);
   SdpParseError error;
   bool ret = webrtc::SdpDeserialize(bad_sdp, &desc, &error);
-  EXPECT_FALSE(ret);
-  EXPECT_NE(std::string::npos, error.line.find(bad_part.c_str()));
+  ASSERT_FALSE(ret);
+  EXPECT_NE(std::string::npos, error.line.find(bad_part.c_str()))
+      << "Did not find " << bad_part << " in " << error.line;
 }
 
 // Expect fail to parse kSdpFullString if replace |good_part| with |bad_part|.
@@ -4775,3 +4776,10 @@
   JsepSessionDescription jdesc_output(kDummyType);
   EXPECT_TRUE(SdpDeserialize(sdp, &jdesc_output));
 }
+
+TEST_F(WebRtcSdpTest, IllegalMidCharacterValue) {
+  std::string sdp = kSdpString;
+  // [ is an illegal token value.
+  Replace("a=mid:", "a=mid:[]", &sdp);
+  ExpectParseFailure(std::string(sdp), "a=mid:[]");
+}