From b0a9f8cf16b2fade13b26430eee80b2a1d1cb561 Mon Sep 17 00:00:00 2001 From: ubeddulla khan Date: Thu, 23 Jul 2026 15:08:12 +0530 Subject: [PATCH 1/2] cap simple string length in RedisReply::ConsumePartialIOBuf Signed-off-by: ubeddulla khan --- src/brpc/redis_reply.cpp | 5 +++++ test/brpc_redis_unittest.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/brpc/redis_reply.cpp b/src/brpc/redis_reply.cpp index 14c76f4841..ad9e66b930 100644 --- a/src/brpc/redis_reply.cpp +++ b/src/brpc/redis_reply.cpp @@ -141,6 +141,11 @@ ParseError RedisReply::ConsumePartialIOBuf(butil::IOBuf& buf, int depth) { return PARSE_ERROR_NOT_ENOUGH_DATA; } const size_t len = str.size() - 1; + if (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 9095c82961..daf06224ee 100644 --- a/test/brpc_redis_unittest.cpp +++ b/test/brpc_redis_unittest.cpp @@ -1499,6 +1499,34 @@ 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); + } + // Test redis_command.cpp limits { // Test command string exceeding limit From aa88be72909498646fd4ed05f65bba01fa37490d Mon Sep 17 00:00:00 2001 From: ubeddulla khan Date: Wed, 29 Jul 2026 11:42:59 +0530 Subject: [PATCH 2/2] reject negative redis_max_allocation_size in simple string branch Signed-off-by: ubeddulla khan --- src/brpc/redis_reply.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/brpc/redis_reply.cpp b/src/brpc/redis_reply.cpp index ad9e66b930..86d702b117 100644 --- a/src/brpc/redis_reply.cpp +++ b/src/brpc/redis_reply.cpp @@ -141,7 +141,8 @@ ParseError RedisReply::ConsumePartialIOBuf(butil::IOBuf& buf, int depth) { return PARSE_ERROR_NOT_ENOUGH_DATA; } const size_t len = str.size() - 1; - if (len > (size_t)FLAGS_redis_max_allocation_size) { + 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;