ubeddulla commented on code in PR #3381:
URL: https://github.com/apache/brpc/pull/3381#discussion_r3568202227
##########
src/brpc/policy/mysql/mysql_reply.cpp:
##########
@@ -830,6 +830,16 @@ ParseError MysqlReply::Field::Parse(butil::IOBuf& buf,
set_parsed();
return PARSE_OK;
}
+ // The length-encoded value must fit in the remaining buffer. cutn clamps
to
+ // what is available, so an oversized len leaves the tail of the len-byte
+ // allocation uninitialized while _data.str is published as len bytes,
+ // exposing uninitialized arena memory and desyncing the packet stream.
+ // The binary Field::Parse and Column::Parse paths already guard this.
+ if (len > buf.size()) {
+ LOG(WARNING) << "MysqlReply::Field::Parse: field length " << len
+ << " exceeds remaining buffer size " << buf.size();
+ return PARSE_ERROR_ABSOLUTELY_WRONG;
+ }
Review Comment:
Good call, added one in test/brpc_mysql_reply_parse_unittest.cpp. It crafts
a text result set whose single row field uses a 0xFD lenenc prefix claiming
0xFFFFFF bytes with nothing left in the buffer, and asserts ConsumePartialIOBuf
returns PARSE_ERROR_ABSOLUTELY_WRONG. There's also a well-formed-field case so
the guard doesn't reject legitimate rows.
--
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]
