ubeddulla commented on code in PR #3379:
URL: https://github.com/apache/brpc/pull/3379#discussion_r3568416796
##########
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:
Good points. Switched it to LOG_EVERY_SECOND so a peer replaying crafted
frames can't spam the log, and gave it its own message so the overlong/shift
case reads differently from the MAX_HPACK_INTEGER failure below it.
--
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]
