Copilot commented on code in PR #3381:
URL: https://github.com/apache/brpc/pull/3381#discussion_r3566267240
##########
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:
Consider adding a regression test that crafts a truncated text-protocol row
field where the length-encoded prefix claims more bytes than are available, and
assert that the parser returns PARSE_ERROR_ABSOLUTELY_WRONG (and does not
advance/desync the buffer). This change hardens the parser, but the new
rejection path is currently untested.
--
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]