This is an automated email from the ASF dual-hosted git repository.
wwbmmm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 24146ca5 cap simple string length in RedisReply::ConsumePartialIOBuf
(#3404)
24146ca5 is described below
commit 24146ca5565e07a29cfbda23595e1796bf00d355
Author: UB <[email protected]>
AuthorDate: Sat Aug 15 11:42:00 2026 +0530
cap simple string length in RedisReply::ConsumePartialIOBuf (#3404)
* cap simple string length in RedisReply::ConsumePartialIOBuf
Signed-off-by: ubeddulla khan <[email protected]>
* reject negative redis_max_allocation_size in simple string branch
Signed-off-by: ubeddulla khan <[email protected]>
* enforce redis simple string cap while waiting for CRLF
Signed-off-by: ubeddulla khan <[email protected]>
---------
Signed-off-by: ubeddulla khan <[email protected]>
---
src/brpc/redis_reply.cpp | 18 +++++++++++++
test/brpc_redis_unittest.cpp | 61 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/src/brpc/redis_reply.cpp b/src/brpc/redis_reply.cpp
index 14c76f48..256e43c0 100644
--- a/src/brpc/redis_reply.cpp
+++ b/src/brpc/redis_reply.cpp
@@ -138,9 +138,27 @@ ParseError RedisReply::ConsumePartialIOBuf(butil::IOBuf&
buf, int depth) {
" actually=" << len;
return PARSE_ERROR_ABSOLUTELY_WRONG;
}
+ // Enforce the cap while still waiting for CRLF, otherwise a peer
+ // that never sends the terminator can grow buf without bound (like
+ // RedisCommandParser does for inline commands). buf holds the
first
+ // char plus the payload so far; allow one extra byte for a
boundary
+ // '\r' whose matching '\n' hasn't arrived yet.
+ if (FLAGS_redis_max_allocation_size < 0 ||
+ len > (size_t)FLAGS_redis_max_allocation_size + 2) {
+ LOG(ERROR) << "simple string exceeds max allocation size! max="
+ << FLAGS_redis_max_allocation_size
+ << ", actually=" << len - 1;
+ return PARSE_ERROR_ABSOLUTELY_WRONG;
+ }
return PARSE_ERROR_NOT_ENOUGH_DATA;
}
const size_t len = str.size() - 1;
+ if (FLAGS_redis_max_allocation_size < 0 ||
+ len > (size_t)FLAGS_redis_max_allocation_size) {
+ LOG(ERROR) << "simple string exceeds max allocation size! max="
+ << FLAGS_redis_max_allocation_size << ", actually=" <<
len;
+ return PARSE_ERROR_ABSOLUTELY_WRONG;
+ }
if (len < sizeof(_data.short_str)) {
// SSO short strings, including empty string.
_type = (fc == '-' ? REDIS_REPLY_ERROR : REDIS_REPLY_STATUS);
diff --git a/test/brpc_redis_unittest.cpp b/test/brpc_redis_unittest.cpp
index 9095c829..dc0f9d55 100644
--- a/test/brpc_redis_unittest.cpp
+++ b/test/brpc_redis_unittest.cpp
@@ -1499,6 +1499,67 @@ TEST_F(RedisTest, memory_allocation_limits) {
ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, err);
}
+ {
+ // Simple string exceeding limit. Unlike bulk strings and arrays this
+ // branch had no cap, so a length >= 2^31 truncated the signed _length
+ // field to a negative value and later reads went out of bounds.
+ butil::IOBuf buf;
+ std::string large_status = "+";
+ large_status.append(2000, 'a');
+ large_status.append("\r\n");
+ buf.append(large_status);
+
+ brpc::RedisReply reply(&arena);
+ brpc::ParseError err = reply.ConsumePartialIOBuf(buf);
+ ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, err);
+ }
+
+ {
+ // Error string exceeding limit (same branch as simple string).
+ butil::IOBuf buf;
+ std::string large_error = "-";
+ large_error.append(2000, 'a');
+ large_error.append("\r\n");
+ buf.append(large_error);
+
+ brpc::RedisReply reply(&arena);
+ brpc::ParseError err = reply.ConsumePartialIOBuf(buf);
+ ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, err);
+ }
+
+ {
+ // Simple string exceeding limit before CRLF arrives. Without a cap on
+ // the waiting-for-CRLF path a peer that never sends the terminator
+ // could grow buf without bound.
+ butil::IOBuf buf;
+ std::string large_status = "+";
+ large_status.append(brpc::FLAGS_redis_max_allocation_size + 100, 'a');
+ buf.append(large_status);
+
+ brpc::RedisReply reply(&arena);
+ brpc::ParseError err = reply.ConsumePartialIOBuf(buf);
+ ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, err);
+ }
+
+ {
+ // A simple string exactly at the limit may have its CRLF split across
+ // reads; a lone trailing '\r' must not trip the cap early.
+ butil::IOBuf buf;
+ std::string boundary_status = "+";
+ boundary_status.append(brpc::FLAGS_redis_max_allocation_size, 'a');
+ boundary_status.push_back('\r');
+ buf.append(boundary_status);
+
+ brpc::RedisReply reply(&arena);
+ brpc::ParseError err = reply.ConsumePartialIOBuf(buf);
+ ASSERT_EQ(brpc::PARSE_ERROR_NOT_ENOUGH_DATA, err);
+
+ buf.push_back('\n');
+ err = reply.ConsumePartialIOBuf(buf);
+ ASSERT_EQ(brpc::PARSE_OK, err);
+ ASSERT_EQ(brpc::FLAGS_redis_max_allocation_size, (int)reply.size());
+ }
+
// Test redis_command.cpp limits
{
// Test command string exceeding limit
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
