Copilot commented on code in PR #3451:
URL: https://github.com/apache/brpc/pull/3451#discussion_r3788909500
##########
src/mcpack2pb/parser-inl.h:
##########
@@ -164,6 +164,13 @@ inline void ArrayIterator::init(InputStream* stream,
size_t size) {
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.
+ if (_item_count > left_size()) {
+ _item_count = static_cast<uint32_t>(left_size());
+ }
Review Comment:
The clamp uses left_size(), which is computed as _expected_popped_end -
_expected_popped_bytes. If the array field’s declared payload size is smaller
than sizeof(ItemsHead), left_size() underflows to a huge value, so the clamp
won’t trigger. In that case cut_packed_pod(&items_head) also reads past the
array’s declared boundary, and generated code may still call
Reserve(it.item_count()) with an attacker-controlled huge count. Add an
explicit size >= sizeof(ItemsHead) validation before reading ItemsHead, and
clamp using a non-underflowing max count derived from the declared payload size
(size - sizeof(ItemsHead)).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
