[mysqlnd] Fix result set field metadata length buffer over-read - #23496
Open
iliaal wants to merge 1 commit into
Open
[mysqlnd] Fix result set field metadata length buffer over-read#23496iliaal wants to merge 1 commit into
iliaal wants to merge 1 commit into
Conversation
iliaal
requested review from
SakiTakamachi,
bukka and
kamil-tekiela
as code owners
August 29, 2026 12:02
Comment on lines
+1179
to
+1182
| BAIL_IF_NO_MORE_DATA; \ | ||
| if (UNEXPECTED(len > (zend_ulong)(packet->header.size - (size_t)(p - begin)))) { \ | ||
| goto premature_end; \ | ||
| } \ |
Member
There was a problem hiding this comment.
Suggested change
| BAIL_IF_NO_MORE_DATA; \ | |
| if (UNEXPECTED(len > (zend_ulong)(packet->header.size - (size_t)(p - begin)))) { \ | |
| goto premature_end; \ | |
| } \ | |
| BAIL_IF_NO_MORE_DATA; \ | |
| if (UNEXPECTED(len > (zend_ulong)(packet->header.size - (size_t)(p - begin)))) { \ | |
| php_error_docref(NULL, E_WARNING, "Result set field metadata string length is past the packet size"); \ | |
| DBG_RETURN(FAIL); \ | |
| } \ |
This part is untested, but if there was a test the value in the test would underflow, so it's probably better to do it how we do it on line 740. Also, on line 740 we don't do (zend_ulong) cast so I think that's redundant too, but I am not sure.
Member
Author
There was a problem hiding this comment.
Matched the line 740 check and warning. goto premature_end underflows the reported size when p is still inside the packet; added mysqlnd_rset_field_len_past_packet.phpt for that.
The rset_field metadata reader trusted each length-encoded string size: a hostile server could send a length marker at the end of a field packet whose value exceeds the remaining payload, advancing p past header.size and past the command buffer before the next dereference, and recording attacker-controlled lengths on pointers outside the packet that later feed memcpy() into the field memory pool. Bound each metadata string by bailing once p leaves the payload and rejecting lengths larger than the remaining bytes, matching php_mysqlnd_auth_response_read() from GHSA-h35g-vwh6-m678; an audit found no other users of the READ_RSET_FIELD macro and the trailing default-value check never dereferences its length.
iliaal
force-pushed
the
fix/rset-field-len-overread-84
branch
from
August 29, 2026 14:31
907fde5 to
ef091dc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In php_mysqlnd_rset_field_read(), a hostile server can place a length marker at the end of a metadata packet whose value exceeds the remaining payload, advancing p past header.size and the command buffer and attaching attacker-controlled lengths to pointers outside the packet that feed memcpy(). Metadata strings now bail out once p leaves the payload and reject lengths beyond the remaining bytes. No other READ_RSET_FIELD users exist and the trailing default-value check never dereferences its length. A hostile-server phpt fails unpatched with an extra protocol-length warning and passes patched.