cap simple string length in RedisReply::ConsumePartialIOBuf

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);

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