Skip to content

check field length against remaining buffer in mysql Field::Parse#3381

Open
ubeddulla wants to merge 1 commit into
apache:masterfrom
ubeddulla:mysql-field-length-bound
Open

check field length against remaining buffer in mysql Field::Parse#3381
ubeddulla wants to merge 1 commit into
apache:masterfrom
ubeddulla:mysql-field-length-bound

Conversation

@ubeddulla

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: resolve N/A

Problem Summary:

The text-protocol MysqlReply::Field::Parse in src/brpc/policy/mysql/mysql_reply.cpp reads a length-encoded field length from the server and calls buf.cutn(&str, len) without checking len against the remaining buffer. IOBuf::cutn clamps to what is available, so a field whose length-encoded prefix claims more bytes than remain makes the string branch allocate len arena bytes, copy only the fewer bytes actually present, and then publish _data.str as a StringPiece of len bytes. The tail is uninitialized arena memory that gets read back by the caller, and the short read desyncs the packet stream. A malicious or compromised MySQL server can trigger it through any text result set. Every sibling length-encoded read in the same file (the binary Field::Parse, Column::Parse, the auth-plugin path, ParseBinaryTime/ParseBinaryDataTime) already rejects len > buf.size(); only this text overload was missing the check.

What is changed and the side effects?

Changed:

Added the same len > buf.size() guard before the cutn in the text Field::Parse, returning PARSE_ERROR_ABSOLUTELY_WRONG on an oversized length. Valid result sets are unaffected since a well-formed field always fits in the buffer.

Side effects:

  • Performance effects: none, a single size comparison per field.

  • Breaking backward compatibility: none.


Check List:

Signed-off-by: ubeddulla khan <ubed@bugqore.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant