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]