From 99bcf60a41206a3c1c9b9d7608e8eac5f32ddbdd Mon Sep 17 00:00:00 2001
From: Harald Alvestrand <hta@webrtc.org>
Date: Wed, 3 Mar 2021 07:44:39 +0000
Subject: [PATCH] Check MID for illegal token characters.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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          | 27 ++++++++++++++++++++++++++-
 pc/webrtc_sdp_unittest.cc | 12 ++++++++++--
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index c4ebf597fe..26eb4f30fd 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -107,6 +107,15 @@ namespace webrtc {
 // 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 @@ static bool GetValue(const std::string& message,
   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 @@ bool ParseContent(const std::string& message,
       // 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;
diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 296781f202..5e7c225aa1 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -951,8 +951,9 @@ static void ExpectParseFailure(const std::string& bad_sdp,
   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 @@ TEST_F(WebRtcSdpTest, SctpPortInUnsupportedContent) {
   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:[]");
+}