High watch CVSS 8.8 chrome OOB 📄 Reporter bug report 🔧 Commit mapped

Overview

High
Severity
8.8
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactOut of bounds write in WebRTC
DescriptionOut of bounds write in WebRTC
ComponentWebRTC
Bug ClassOOB
Action tierwatch — corruption primitive but gated (uncommon) — not default-reachable
Computed by PatchGap's deterministic exploitability ruler from the fixing commit. patch-now = RCE-class & attacker-reachable in a default config, or exploited in the wild; watch = real security signal below that bar; noise = no security relevance.
Tracker486536241
Fix commit5e177d217195 (src) +13/-54
CWECWE-787 (Out-of-bounds write)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H
CISA KEVNot listed
Creditedc6eed09fc8b174b0f3eebedcceb1e792
Reported2026-02-22 (per vendor advisory)
Fix landed2026-03-25 (public git)
Disclosed2026-05-12
Patch gap48 days — fix public in source before the advisory

Background

`SimpleStringBuilder`
a WebRTC helper that formats text into a caller-supplied fixed std::span<char> buffer (often a stack array), with no dynamic allocation.
`StringBuilder`
the heap-backed sibling that formats into an internally owned std::string that grows as needed; its Release() moves the string out.
`RTC_DCHECK` vs `RTC_CHECK`
DCHECK variants are compiled out in release/production builds, so their condition is only enforced in debug; CHECK variants are always enforced and abort on failure.
`SafeMin`
a clamp helper used before memcpy to limit how many bytes are copied to the remaining buffer space.

Root Cause Analysis

SimpleStringBuilder guarded its buffer-capacity invariants with RTC_DCHECK/RTC_DCHECK_LT/RTC_DCHECK_EQ, so in the release builds Chrome and WebRTC ship these bounds checks never executed; the commit message states this led to “undefined behavior or silent truncation.” The operator<<(absl::string_view) body shown does clamp its own memcpy with SafeMin(str.length(), buffer_.size() - size_ - 1), so that specific copy is bounded — but the clamp relies on buffer_.size() - size_ - 1 not having underflowed, and the always-on invariant that the DCHECKs were supposed to enforce was absent in release. The constructor writes buffer_[0] = '\0' before any check, and the AppendFormat/vsnprintf path (hunk at line ~98, whose enclosing function name is not shown in the diff) reconciles len against remaining space only via a DCHECK_EQ, so an undersized buffer silently truncated rather than being caught.

The fix promotes every one of these to RTC_CHECK, so a buffer that is too small now aborts deterministically in all build configurations instead of silently truncating or risking out-of-bounds behavior. Separately, the patch removes the fixed char buf[256] stack buffer from RtpExtension::ToString and switches it to the heap-backed StringBuilder, eliminating the fixed-capacity constraint at an attacker-reachable call site.

Key insight
The core mistake was enforcing buffer-capacity safety only with debug-only RTC_DCHECK, leaving release builds to silently truncate (or risk out-of-bounds writes) when input exceeded the fixed buffer; the fix makes the checks always-on RTC_CHECK and moves the string-building call sites off fixed stack buffers onto the growable StringBuilder.

Attack Path

  1. Reach a fixed-buffer formatter with oversized input Supply input that drives a SimpleStringBuilder with a small fixed buffer past its capacity; RtpExtension::ToString formatted into char buf[256], and the uri field it appends is influenced by remote SDP (inferred: remote peer control of RTP extension URIs).
  2. Debug checks absent in release In production, the RTC_DCHECK/RTC_DCHECK_LT/RTC_DCHECK_EQ guarding capacity are compiled out, so the overflow/truncation condition is not caught.
  3. Trigger the unsafe size accounting When accumulated size_ meets or exceeds buffer_.size(), the unsigned expression buffer_.size() - size_ - 1 underflows to a huge value, defeating the SafeMin clamp and allowing an oversized memcpy (out-of-bounds write). The exact overflowing sink is not fully visible in the shown hunks (the vsnprintf fill in AppendFormat is above the diff window) (inferred).
  4. Patched outcome With RTC_CHECK, the same condition now aborts the process deterministically, and RtpExtension::ToString no longer uses a fixed buffer at all.

Impact Assessment

The advisory classifies this as an out of bounds write in WebRTC, rated high. An attacker able to feed over-length data into a SimpleStringBuilder fixed buffer (e.g. via RTP extension URIs reaching RtpExtension::ToString) could, in release builds, cause silent truncation or an out-of-bounds write within the process handling WebRTC. The diff demonstrates the missing always-on bounds enforcement and removes a fixed stack buffer; any further escalation to control-flow hijack or RCE is not shown by the patch and is (inferred).

Changed Functions

FunctionChangeNotes
if
api/rtp_parameters.cc
modified
if
rtc_base/strings/string_builder.cc
modified
TEST
rtc_base/strings/string_builder_unittest.cc
modified

Files Changed

  • api/rtp_parameters.cc
  • rtc_base/BUILD.gn
  • rtc_base/strings/string_builder.cc
  • rtc_base/strings/string_builder.h
  • rtc_base/strings/string_builder_unittest.cc

Audit Directions

  • DCHECK-guarded memory safety
    Search for RTC_DCHECK/DCHECK (and _LT/_EQ/_GE variants) that guard buffer sizes, indices, or memcpy/vsnprintf lengths; any such check that is the only thing standing between input and an out-of-bounds access is a release-build hole.
  • Fixed-size stack buffers with external input
    Flag SimpleStringBuilder (or raw char buf[N]) used to format strings that incorporate remote/negotiated data (SDP fields, URIs, names); prefer the growable StringBuilder.
  • Unsigned size arithmetic before clamps
    Review capacity - used - 1 style expressions feeding SafeMin/memcpy; verify used can never meet or exceed capacity, since underflow turns a clamp into an overflow.
From 5e177d217195edbcdf1f54f928a942e1696197d8 Mon Sep 17 00:00:00 2001
From: Tommi <[email protected]>
Date: Tue, 24 Mar 2026 15:22:38 +0100
Subject: [PATCH] Harden SimpleStringBuilder safety checks

Replace RTC_DCHECK with RTC_CHECK in SimpleStringBuilder to ensure
safety constraints are enforced in all build configurations, including
release builds. This change prevents undefined behavior or silent
truncation by crashing when a buffer overflow is detected, rather than
only performing these checks in debug modes.

The modifications include:
* Updating SimpleStringBuilder to use RTC_CHECK for consistency and
  bounds verification.
* Simplifying RtpExtension::ToString and VideoReceiveStream2 by
  migrating from SimpleStringBuilder to StringBuilder.
* Simplify+clarify the StringBuilder::Release implementation.
* Update unit tests to reflect that overflow now consistently results
  in a crash.

Bug: chromium:486536241
Change-Id: Ia067508e2b9409154ad4afdbe4c0971a1dd32e1e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/459441
Commit-Queue: Tomas Gunnarsson <[email protected]>
Reviewed-by: Per Kjellander <[email protected]>
Cr-Commit-Position: refs/heads/main@{#47251}
---

diff --git a/api/rtp_parameters.cc b/api/rtp_parameters.cc
index 3a36bfe..b6b2f89 100644
--- a/api/rtp_parameters.cc
+++ b/api/rtp_parameters.cc
@@ -18,7 +18,6 @@
 
 #include "absl/strings/ascii.h"
 #include "absl/strings/string_view.h"
-#include "api/array_view.h"
 #include "api/rtc_error.h"
 #include "api/rtp_transceiver_direction.h"
 #include "media/base/media_constants.h"
@@ -206,15 +205,14 @@
 RtpParameters::~RtpParameters() = default;
 
 std::string RtpExtension::ToString() const {
-  char buf[256];
-  SimpleStringBuilder sb(buf);
+  StringBuilder sb;
   sb << "{uri: " << uri;
   sb << ", id: " << id;
   if (encrypt) {
     sb << ", encrypt";
   }
-  sb << '}';
-  return sb.str();
+  sb << "}";
+  return sb.Release();
 }
 
 bool RtpExtension::IsSupportedForAudio(absl::string_view uri) {
diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index b98dcd6..29888f0 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -2184,6 +2184,7 @@
         "network:received_packet",
         "synchronization:mutex",
         "task_utils:repeating_task",
+        "//testing/gtest",
         "//third_party/abseil-cpp/absl/algorithm:container",
         "//third_party/abseil-cpp/absl/base:core_headers",
         "//third_party/abseil-cpp/absl/memory",
diff --git a/rtc_base/strings/string_builder.cc b/rtc_base/strings/string_builder.cc
index f52445a..af8e7e8 100644
--- a/rtc_base/strings/string_builder.cc
+++ b/rtc_base/strings/string_builder.cc
@@ -24,7 +24,7 @@
 SimpleStringBuilder::SimpleStringBuilder(std::span<char> buffer)
     : buffer_(buffer) {
   buffer_[0] = '\0';
-  RTC_DCHECK(IsConsistent());
+  RTC_CHECK(IsConsistent());
 }
 
 SimpleStringBuilder& SimpleStringBuilder::operator<<(char ch) {
@@ -32,13 +32,13 @@
 }
 
 SimpleStringBuilder& SimpleStringBuilder::operator<<(absl::string_view str) {
-  RTC_DCHECK_LT(size_ + str.length(), buffer_.size())
+  RTC_CHECK_LT(size_ + str.length(), buffer_.size())
       << "Buffer size was insufficient";
   const size_t chars_added = SafeMin(str.length(), buffer_.size() - size_ - 1);
   memcpy(&buffer_[size_], str.data(), chars_added);
   size_ += chars_added;
   buffer_[size_] = '\0';
-  RTC_DCHECK(IsConsistent());
+  RTC_CHECK(IsConsistent());
   return *this;
 }
 
@@ -98,7 +98,7 @@
   if (len >= 0) {
     const size_t chars_added = SafeMin(len, buffer_.size() - 1 - size_);
     size_ += chars_added;
-    RTC_DCHECK_EQ(len, chars_added) << "Buffer size was insufficient";
+    RTC_CHECK_EQ(len, chars_added) << "Buffer size was insufficient";
   } else {
     // This should never happen, but we're paranoid, so re-write the
     // terminator in case vsnprintf() overwrote it.
@@ -106,7 +106,7 @@
     buffer_[size_] = '\0';
   }
   va_end(args);
-  RTC_DCHECK(IsConsistent());
+  RTC_CHECK(IsConsistent());
   return *this;
 }
 
diff --git a/rtc_base/strings/string_builder.h b/rtc_base/strings/string_builder.h
index 41efe9a..4a3df25 100644
--- a/rtc_base/strings/string_builder.h
+++ b/rtc_base/strings/string_builder.h
@@ -155,11 +155,8 @@
 
   size_t size() const { return str_.size(); }
 
-  std::string Release() {
-    std::string ret = std::move(str_);
-    str_.clear();
-    return ret;
-  }
+  // Moves out the internal std::string.
+  std::string Release() { return std::move(str_); }
 
   // Allows appending a printf style formatted string.
   StringBuilder& AppendFormat(const char* fmt, ...)
diff --git a/rtc_base/strings/string_builder_unittest.cc b/rtc_base/strings/string_builder_unittest.cc
index cb45313..4764ac4 100644
--- a/rtc_base/strings/string_builder_unittest.cc
+++ b/rtc_base/strings/string_builder_unittest.cc
@@ -14,7 +14,6 @@
 #include <string>
 
 #include "absl/strings/string_view.h"
-#include "rtc_base/checks.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 
@@ -81,18 +80,13 @@
 
 // These tests are safe to run if we have death test support or if DCHECKs are
 // off.
-#if (GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)) || !RTC_DCHECK_IS_ON
+#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunConstCharP) {
   char sb_buf[4];
   SimpleStringBuilder sb(sb_buf);
   const char* const msg = "This is just too much";
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << msg, "");
-#else
-  sb << msg;
-  EXPECT_THAT(sb.str(), ::testing::StrEq("Thi"));
-#endif
 }
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunStdString) {
@@ -100,41 +94,21 @@
   SimpleStringBuilder sb(sb_buf);
   sb << 12;
   const std::string msg = "Aw, come on!";
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << msg, "");
-#else
-  sb << msg;
-  EXPECT_THAT(sb.str(), ::testing::StrEq("12A"));
-#endif
 }
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunInt) {
   char sb_buf[4];
   SimpleStringBuilder sb(sb_buf);
   constexpr int num = -12345;
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << num, "");
-#else
-  sb << num;
-  // If we run into the end of the buffer, resonable results are either that
-  // the append has no effect or that it's truncated at the point where the
-  // buffer ends.
-  EXPECT_THAT(sb.str(),
-              ::testing::AnyOf(::testing::StrEq(""), ::testing::StrEq("-12")));
-#endif
 }
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunDouble) {
   char sb_buf[5];
   SimpleStringBuilder sb(sb_buf);
   constexpr double num = 123.456;
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/rtc_base/strings/string_builder_unittest.cc b/rtc_base/strings/string_builder_unittest.cc
index cb45313..4764ac4 100644
--- a/rtc_base/strings/string_builder_unittest.cc
+++ b/rtc_base/strings/string_builder_unittest.cc
@@ -14,7 +14,6 @@
 #include <string>
 
 #include "absl/strings/string_view.h"
-#include "rtc_base/checks.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 
@@ -81,18 +80,13 @@
 
 // These tests are safe to run if we have death test support or if DCHECKs are
 // off.
-#if (GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)) || !RTC_DCHECK_IS_ON
+#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunConstCharP) {
   char sb_buf[4];
   SimpleStringBuilder sb(sb_buf);
   const char* const msg = "This is just too much";
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << msg, "");
-#else
-  sb << msg;
-  EXPECT_THAT(sb.str(), ::testing::StrEq("Thi"));
-#endif
 }
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunStdString) {
@@ -100,41 +94,21 @@
   SimpleStringBuilder sb(sb_buf);
   sb << 12;
   const std::string msg = "Aw, come on!";
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << msg, "");
-#else
-  sb << msg;
-  EXPECT_THAT(sb.str(), ::testing::StrEq("12A"));
-#endif
 }
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunInt) {
   char sb_buf[4];
   SimpleStringBuilder sb(sb_buf);
   constexpr int num = -12345;
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << num, "");
-#else
-  sb << num;
-  // If we run into the end of the buffer, resonable results are either that
-  // the append has no effect or that it's truncated at the point where the
-  // buffer ends.
-  EXPECT_THAT(sb.str(),
-              ::testing::AnyOf(::testing::StrEq(""), ::testing::StrEq("-12")));
-#endif
 }
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunDouble) {
   char sb_buf[5];
   SimpleStringBuilder sb(sb_buf);
   constexpr double num = 123.456;
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << num, "");
-#else
-  sb << num;
-  EXPECT_THAT(sb.str(),
-              ::testing::AnyOf(::testing::StrEq(""), ::testing::StrEq("123.")));
-#endif
 }
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunConstCharPAlreadyFull) {
@@ -142,12 +116,7 @@
   SimpleStringBuilder sb(sb_buf);
   sb << 123;
   const char* const msg = "This is just too much";
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << msg, "");
-#else
-  sb << msg;
-  EXPECT_THAT(sb.str(), ::testing::StrEq("123"));
-#endif
 }
 
 TEST(SimpleStringBuilderDeathTest, BufferOverrunIntAlreadyFull) {
@@ -155,12 +124,7 @@
   SimpleStringBuilder sb(sb_buf);
   sb << "xyz";
   constexpr int num = -12345;
-#if RTC_DCHECK_IS_ON
   EXPECT_DEATH(sb << num, "");
-#else
-  sb << num;
-  EXPECT_THAT(sb.str(), ::testing::StrEq("xyz"));
-#endif
 }
 
 #endif
Loading diff…

Original Bug Report

reported by [email protected]

Missing bounds check in GetSendEncodingsFromRemoteDescription leads to out-of-bounds write in VideoCodecInitializer::SetupCodec via remote SDP simulcast RID injection

deleted
View on issue tracker