This is an automated email from the ASF dual-hosted git repository.

chenBright 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 437a7b70 Limit mcpack2pb array item count to the actual payload size 
(#3451)
437a7b70 is described below

commit 437a7b705ccab4a5c7d8be64acc7cefdb150ad79
Author: Weibing Wang <[email protected]>
AuthorDate: Sun Aug 16 01:06:46 2026 +0800

    Limit mcpack2pb array item count to the actual payload size (#3451)
    
    * Limit mcpack2pb array item count to the actual payload size
    
    The item count in an mcpack array header is read directly from the
    request and was used as-is by the generated parsing code to Reserve()
    memory for repeated protobuf fields. A malformed request could claim an
    item count up to INT32_MAX and force the server to preallocate ~16GB of
    virtual memory, which may abort the process on memory-constrained hosts.
    
    Cap the item count by the remaining bytes of the array (each item
    occupies at least one byte) so that the preallocation is bounded by the
    request size.
    
    * Fix underflow in mcpack2pb array item count clamping
---
 src/mcpack2pb/parser-inl.h       | 16 ++++++++
 test/brpc_mcpack2pb_unittest.cpp | 79 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/src/mcpack2pb/parser-inl.h b/src/mcpack2pb/parser-inl.h
index 76d03fea..235bb540 100644
--- a/src/mcpack2pb/parser-inl.h
+++ b/src/mcpack2pb/parser-inl.h
@@ -158,12 +158,24 @@ inline void ArrayIterator::init(InputStream* stream, 
size_t size) {
     _stream = stream;
     _expected_popped_bytes = _stream->popped_bytes() + sizeof(ItemsHead);
     _expected_popped_end = _stream->popped_bytes() + size;
+    if (size < sizeof(ItemsHead)) {
+        CHECK(false) << "buffer(size=" << size << ") is not enough";
+        return set_bad();
+    }
     ItemsHead items_head;
     if (_stream->cut_packed_pod(&items_head) != sizeof(ItemsHead)) {
         CHECK(false) << "buffer(size=" << size << ") is not enough";
         return set_bad();
     }
     _item_count = items_head.item_count;
+    // The item count is read from the request and may be much larger than
+    // the actual payload. The generated code uses it to Reserve() memory for
+    // repeated protobuf fields, so cap it by the remaining bytes (each item
+    // occupies at least one byte) to avoid a huge preallocation.
+    const size_t remaining = size - sizeof(ItemsHead);
+    if (_item_count > remaining) {
+        _item_count = static_cast<uint32_t>(remaining);
+    }
     operator++();
 }
 
@@ -175,6 +187,10 @@ inline void ISOArrayIterator::init(InputStream* stream, 
size_t size) {
     _item_size = 0;
     _item_count = 0;
     _left_item_count = 0;
+    if (size < sizeof(IsoItemsHead)) {
+        CHECK(false) << "Not enough data";
+        return set_bad();
+    }
     IsoItemsHead items_head;
     if (_stream->cut_packed_pod(&items_head) != sizeof(IsoItemsHead)) {
         CHECK(false) << "Not enough data";
diff --git a/test/brpc_mcpack2pb_unittest.cpp b/test/brpc_mcpack2pb_unittest.cpp
new file mode 100644
index 00000000..68de0522
--- /dev/null
+++ b/test/brpc_mcpack2pb_unittest.cpp
@@ -0,0 +1,79 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Unit tests for the mcpack2pb parser.
+
+#include <gtest/gtest.h>
+#include "butil/iobuf.h"
+#include "mcpack2pb/parser.h"
+
+namespace {
+
+TEST(Mcpack2pbParserTest, ArrayItemCountIsCappedToRemainingBytes) {
+    // An mcpack array whose header claims item_count = 0x7fffffff (INT32_MAX)
+    // but contains no actual items. The raw item_count is fed by the
+    // generated code into Reserve() of a repeated protobuf field, which used
+    // to preallocate ~16GB from a tiny request. The item count must be capped
+    // by the bytes actually available in the array.
+    const unsigned char data[] = {
+        0xff, 0xff, 0xff, 0x7f,  // item_count = 0x7fffffff, no items
+    };
+    butil::IOBuf body;
+    body.append(data, sizeof(data));
+
+    butil::IOBufAsZeroCopyInputStream zc_stream(body);
+    mcpack2pb::InputStream stream(&zc_stream);
+    mcpack2pb::ArrayIterator it(&stream, sizeof(data));
+    // No item can fit in an empty payload.
+    EXPECT_EQ(0u, it.item_count());
+}
+
+TEST(Mcpack2pbParserTest, ArrayItemCountIsCappedToAvailableBytes) {
+    // item_count = 1000 but the payload only holds one int32 item (6 bytes).
+    // Each item occupies at least one byte, so the count must not exceed the
+    // remaining bytes (6).
+    const unsigned char data[] = {
+        0xe8, 0x03, 0x00, 0x00,  // item_count = 1000
+        0x14, 0x00, 0x00, 0x00, 0x00, 0x00,  // one int32 item
+    };
+    butil::IOBuf body;
+    body.append(data, sizeof(data));
+
+    butil::IOBufAsZeroCopyInputStream zc_stream(body);
+    mcpack2pb::InputStream stream(&zc_stream);
+    mcpack2pb::ArrayIterator it(&stream, sizeof(data));
+    EXPECT_LE(it.item_count(), sizeof(data) - sizeof(uint32_t));
+}
+
+TEST(Mcpack2pbParserTest, ArrayItemCountIsZeroWhenPayloadSmallerThanHeader) {
+    // The declared array payload (3 bytes) is smaller than the 4-byte
+    // ItemsHead, but the stream still contains data. The parser must not read
+    // past the declared boundary and trust the extra bytes as item_count
+    // (which would feed a huge value into Reserve() again).
+    const unsigned char data[] = {
+        0xff, 0xff, 0xff, 0x7f,  // would be read as item_count = 0x7fffffff
+    };
+    butil::IOBuf body;
+    body.append(data, sizeof(data));
+
+    butil::IOBufAsZeroCopyInputStream zc_stream(body);
+    mcpack2pb::InputStream stream(&zc_stream);
+    mcpack2pb::ArrayIterator it(&stream, 3);  // size = 3 < sizeof(ItemsHead)
+    EXPECT_EQ(0u, it.item_count());
+}
+
+}  // namespace


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to