commit c8abb0469dfb1ea4c5a55a56e897bafebbfdaf8d
author Philipp Hancke <phancke@meta.com> Tue Aug 12 18:32:52 2025
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Aug 14 20:27:19 2025
tree 7227620708b576417864e53d51d9d509a0c3331a
parent d2e6050e9b51c4b18cdcbb6d8d72c26344dbd97d

Reject non-token char in BUNDLE (and other) groups

also modernize code for parsing and serialization. This is a safe change
since such BUNDLE tags need to refer to a mid and non-token-char are not accepted for mid.

Bug: webrtc:42220536
Change-Id: Iddf06e7935a4e7ad0a3897739c851bd9dbfedad8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/404203
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#45341}

pc/webrtc_sdp.cc

diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index 3b3f4f1..a96701a 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -191,7 +191,6 @@
 const char kSessionOriginAddress[] = "127.0.0.1";
 const char kSessionName[] = "s=-";
 const char kTimeDescription[] = "t=0 0";
-const char kAttrGroup[] = "a=group:BUNDLE";
 const char kConnectionNettype[] = "IN";
 const char kConnectionIpv4Addrtype[] = "IP4";
 const char kConnectionIpv6Addrtype[] = "IP6";
@@ -1570,13 +1569,18 @@
                          SdpParseError* error) {
   RTC_DCHECK(desc != nullptr);
 
-  // RFC 5888 and draft-holmberg-mmusic-sdp-bundle-negotiation-00
+  // RFC 5888 and RFC 8843.
   // a=group:BUNDLE video voice
   std::vector<absl::string_view> fields =
       split(line.substr(kLinePrefixLength), kSdpDelimiterSpaceChar);
   std::string semantics;
   if (!GetValue(fields[0], kAttributeGroup, &semantics, error)) {
-    return false;
+    return ParseFailed(line, "Failed to parse group attribute.", error);
+  }
+  for (size_t i = 1; i < fields.size(); ++i) {
+    if (!absl::c_all_of(fields[i], IsTokenChar)) {
+      return ParseFailed(line, "Failed to parse group tag.", error);
+    }
   }
   ContentGroup group(semantics);
   for (size_t i = 1; i < fields.size(); ++i) {
@@ -3242,13 +3246,13 @@
   std::vector<const ContentGroup*> groups =
       desc->GetGroupsByName(GROUP_TYPE_BUNDLE);
   for (const ContentGroup* group : groups) {
-    std::string group_line = kAttrGroup;
     RTC_DCHECK(group != nullptr);
+    InitAttrLine(kAttributeGroup, &os);
+    os << kSdpDelimiterColon << GROUP_TYPE_BUNDLE;
     for (const std::string& content_name : group->content_names()) {
-      group_line.append(" ");
-      group_line.append(content_name);
+      os << " " << content_name;
     }
-    AddLine(group_line, &message);
+    AddLine(os.str(), &message);
   }
 
   // Mixed one- and two-byte header extension.

pc/webrtc_sdp_unittest.cc

diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 1f2e29f..54636fa 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -5096,5 +5096,15 @@
   EXPECT_TRUE(desc->ToString(&serialized));
 }
 
+TEST_F(WebRtcSdpTest, RejectsInvalidCharactersInBundleGroup) {
+  JsepSessionDescription desc(kDummyType);
+  std::string sdp_with_bad_bundle_tag = kSdpFullString;
+  // Inject a "shrug" unicode character.
+  InjectAfter(kSessionTime, "a=group:BUNDLE \u1f937\r\n",
+              &sdp_with_bad_bundle_tag);
+  SdpParseError error;
+  EXPECT_FALSE(SdpDeserialize(sdp_with_bad_bundle_tag, &desc, &error));
+}
+
 }  // namespace
 }  // namespace webrtc