Skip to content

Commit 5508210

Browse files
agrawrohphlax
authored andcommitted
json: fixed an off-by-one write that could corrupted the string null terminator
[CVE-2026-26309](GHSA-56cj-wgg3-x943) Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com> Signed-off-by: Ryan Northey <ryan@synca.io>
1 parent 9b314db commit 5508210

3 files changed

Lines changed: 41 additions & 2 deletions

File tree

changelogs/current.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ bug_fixes:
4545
destroyed. This could cause use-after-free conditions when filter callbacks were invoked on filters that
4646
had already received ``onDestroy()``. The fix ensures that ``decodeHeaders()``, ``decodeData()``,
4747
``decodeTrailers()``, and ``decodeMetadata()`` are blocked after a downstream reset.
48+
- area: json
49+
change: |
50+
Fixed an off-by-one write in ``JsonEscaper::escapeString()`` that could corrupt the string null terminator
51+
when the input string ends with a control character.
4852
4953
removed_config_or_runtime:
5054
# *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

source/common/common/json_escape_string.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,12 @@ class JsonEscaper {
6868
// Print character as unicode hex.
6969
sprintf(&result[position + 1], "u%04x", static_cast<int>(character));
7070
position += 6;
71-
// Overwrite trailing null character.
72-
result[position] = '\\';
71+
// Overwrite trailing null character from `sprintf`, but only if there are more characters
72+
// to process. If this is the last character then we must not write past the end of the
73+
// string.
74+
if (position < result.size()) {
75+
result[position] = '\\';
76+
}
7377
} else {
7478
// All other characters are added as-is.
7579
result[position++] = character;

test/common/common/logger_test.cc

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <cstring>
12
#include <memory>
23
#include <string>
34

@@ -118,6 +119,36 @@ TEST(JsonEscapeTest, Escape) {
118119
expect_json_escape("\x1f", "\\u001f");
119120
}
120121

122+
// Regression test for off-by-one write when control characters appear at the end of input.
123+
TEST(JsonEscapeTest, NulTerminatorIntegrity) {
124+
const auto verify_nul_terminator = [](absl::string_view input) {
125+
std::string escaped = JsonEscaper::escapeString(input, JsonEscaper::extraSpace(input));
126+
127+
// Verify the null terminator is intact.
128+
EXPECT_EQ('\0', escaped.c_str()[escaped.size()])
129+
<< "null terminator corrupted for input ending with control character";
130+
131+
// Verify strlen matches the size. A corrupted null terminator would cause strlen
132+
// to read past the string boundary.
133+
EXPECT_EQ(escaped.size(), std::strlen(escaped.c_str()))
134+
<< "strlen mismatch indicates NUL terminator corruption";
135+
};
136+
137+
// Test control characters at the end of input. These would trigger the buggy code path.
138+
verify_nul_terminator("\x01"); // Single control char.
139+
verify_nul_terminator("\x00"); // Single NUL.
140+
verify_nul_terminator("test\x01"); // Trailing control char.
141+
verify_nul_terminator(absl::string_view("test\x00", 5)); // Trailing NUL.
142+
verify_nul_terminator("\x01\x02"); // Multiple control chars.
143+
verify_nul_terminator("test\x01\x02"); // Multiple trailing control chars.
144+
verify_nul_terminator(std::string(100, 'A') + "\x1f"); // Large string ending with control char.
145+
146+
// Test control characters not at the end. These should always work.
147+
verify_nul_terminator("\x01test"); // Leading control char.
148+
verify_nul_terminator("te\x01st"); // Middle control char.
149+
verify_nul_terminator("\x01test\x02more"); // Multiple control chars in middle.
150+
}
151+
121152
class LoggerCustomFlagsTest : public testing::TestWithParam<spdlog::logger*> {
122153
public:
123154
LoggerCustomFlagsTest() : logger_(GetParam()) {}

0 commit comments

Comments
 (0)