diff --git a/src/brpc/redis_reply.cpp b/src/brpc/redis_reply.cpp index 14c76f4841..86d702b117 100644 --- a/src/brpc/redis_reply.cpp +++ b/src/brpc/redis_reply.cpp @@ -141,6 +141,12 @@ ParseError RedisReply::ConsumePartialIOBuf(butil::IOBuf& buf, int depth) { 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 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