Copilot commented on code in PR #3575:
URL: https://github.com/apache/brpc/pull/3575#discussion_r4151712818
##########
src/brpc/policy/mysql/mysql_reply.cpp:
##########
@@ -458,69 +501,69 @@ ParseError MysqlReply::Auth::Parse(butil::IOBuf& buf,
butil::Arena* arena) {
}
const std::string delim(1, 0x00);
MysqlHeader header;
- if (!parse_header(buf, &header)) {
+ butil::IOBuf payload;
+ if (!parse_header(buf, &header, &payload)) {
return PARSE_ERROR_NOT_ENOUGH_DATA;
}
- buf.cut1((char*)&_protocol);
+ payload.cut1((char*)&_protocol);
{
butil::IOBuf version;
- buf.cut_until(&version, delim);
+ payload.cut_until(&version, delim);
char* d = nullptr;
MY_ALLOC_CHECK(my_alloc_check(arena, version.size(), d));
version.copy_to(d);
_version.set(d, version.size());
}
{
uint8_t tmp[4];
- buf.cutn(tmp, sizeof(tmp));
+ payload.cutn(tmp, sizeof(tmp));
_thread_id = mysql_uint4korr(tmp);
Review Comment:
The new packet boundary prevents this read from borrowing bytes from the
next packet, but its return value is still unchecked. A malformed greeting
ending before the four-byte thread ID leaves `tmp` partially uninitialized and
`Auth::Parse` continues successfully; the later fixed-width reads have the same
problem. Validate each delimiter/fixed-width read and reject a truncated
handshake.
This issue also appears in the following locations of the same file:
- line 649
- line 691
- line 722
- line 812
- line 814
- ...and 1 more
--
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]
