CVE-2026-8526
Overview
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::stringthat grows as needed; itsRelease()moves the string out. - `RTC_DCHECK` vs `RTC_CHECK`
DCHECKvariants are compiled out in release/production builds, so their condition is only enforced in debug;CHECKvariants are always enforced and abort on failure.- `SafeMin`
- a clamp helper used before
memcpyto 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.
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
- Reach a fixed-buffer formatter with oversized input
Supply input that drives a
SimpleStringBuilderwith a small fixed buffer past its capacity;RtpExtension::ToStringformatted intochar buf[256], and theurifield it appends is influenced by remote SDP (inferred: remote peer control of RTP extension URIs). - Debug checks absent in release
In production, the
RTC_DCHECK/RTC_DCHECK_LT/RTC_DCHECK_EQguarding capacity are compiled out, so the overflow/truncation condition is not caught. - Trigger the unsafe size accounting
When accumulated
size_meets or exceedsbuffer_.size(), the unsigned expressionbuffer_.size() - size_ - 1underflows to a huge value, defeating theSafeMinclamp and allowing an oversizedmemcpy(out-of-bounds write). The exact overflowing sink is not fully visible in the shown hunks (thevsnprintffill inAppendFormatis above the diff window) (inferred). - Patched outcome
With
RTC_CHECK, the same condition now aborts the process deterministically, andRtpExtension::ToStringno longer uses a fixed buffer at all.
Impact Assessment
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
| Function | Change | Notes |
|---|---|---|
ifapi/rtp_parameters.cc |
modified | |
ifrtc_base/strings/string_builder.cc |
modified | |
TESTrtc_base/strings/string_builder_unittest.cc |
modified |
Files Changed
api/rtp_parameters.ccrtc_base/BUILD.gnrtc_base/strings/string_builder.ccrtc_base/strings/string_builder.hrtc_base/strings/string_builder_unittest.cc
Audit Directions
- DCHECK-guarded memory safetySearch for
RTC_DCHECK/DCHECK(and_LT/_EQ/_GEvariants) that guard buffer sizes, indices, ormemcpy/vsnprintflengths; 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 inputFlag
SimpleStringBuilder(or rawchar buf[N]) used to format strings that incorporate remote/negotiated data (SDP fields, URIs, names); prefer the growableStringBuilder. - Unsigned size arithmetic before clampsReview
capacity - used - 1style expressions feedingSafeMin/memcpy; verifyusedcan never meet or exceedcapacity, since underflow turns a clamp into an overflow.
Patch
From 5e177d217195edbcdf1f54f928a942e1696197d8 Mon Sep 17 00:00:00 2001
From: Tommi <tommi@webrtc.org>
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 <tommi@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
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;
Regression Test / PoC
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
Original Bug Report
Missing bounds check in GetSendEncodingsFromRemoteDescription leads to out-of-bounds write in VideoCodecInitializer::SetupCodec via remote SDP simulcast RID injection