Copilot commented on code in PR #3379:
URL: https://github.com/apache/brpc/pull/3379#discussion_r3566264603
##########
src/brpc/details/hpack.cpp:
##########
@@ -547,6 +547,16 @@ inline ssize_t DecodeInteger(butil::IOBufBytesIterator&
iter,
if (!iter) {
return 0;
}
+ // A well-formed integer below MAX_HPACK_INTEGER fits in a few
+ // continuation octets. A run of 0x80 octets (continuation bit set,
+ // payload bits zero) leaves tmp unchanged, so the `tmp <
+ // MAX_HPACK_INTEGER` guard below never trips while m keeps growing;
+ // once m reaches 64 the `<< m` shift is undefined behavior. Refuse the
+ // over-long encoding before that happens.
+ if (m >= 32) {
+ LOG(ERROR) << "Source stream is likely malformed";
+ return -1;
+ }
Review Comment:
This new malformed-input path logs at ERROR with a generic message. Since
HPACK decode is reachable from untrusted peers, repeated crafted frames could
spam logs; also this message is indistinguishable from the existing `tmp >=
MAX_HPACK_INTEGER` failure below, which hurts diagnostics. Consider
rate-limiting and making the message specific to the overlong/shift case.
--
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]
