Repository navigation
fix(mysql): harden protocol decoders against malformed packets from hostile servers - #4458
Merged
abonander merged 1 commit intoOct 16, 2026
Conversation
…ostile servers Found during a security review of server-controlled parsing paths (threat model: rogue or MITM MySQL/MariaDB server; handshake parsing is reachable pre-authentication). ### Does your PR solve an issue? No tracking issue — reported here directly. This extends the fix pattern of transact-rs#4176 (truncated `OK` packet) to the rest of the protocol; transact-rs#4451 documents the same review on the PostgreSQL side. ### The problem - `recv_packet`'s continuation loop accumulated an unbounded number of `0xFFFFFF`-sized parts: a hostile server streams endless 16 MiB frames and the client OOMs at near-zero bandwidth cost. - The result-set column count (a length-encoded u64) fed `HashMap::with_capacity` / `Vec::reserve` directly; a ~10-byte packet claiming 2^40 columns aborts the process. - Dozens of decode sites call panicking `bytes::Buf` getters / slice indexing / `advance` on untrusted buffers: truncated ERR packets, every field of `HandshakeV10` (including the attacker-controlled auth-plugin-data length), length-encoded strings claiming more than the remaining bytes, `TextRow` field-count mismatch, `BinaryRow` (null bitmap, temporal length byte, fixed-width and lenenc value bounds — plus an `unreachable!()` actually reachable via a NULL-typed column), short EOF packets, and a 1-byte `caching_sha2_password` continue packet. The OkPacket decoder was hardened in transact-rs#4176; the rest of the protocol was not. ### The change - Cumulative 1 GiB cap on packet reassembly (`MySql`'s own `max_allowed_packet` hard ceiling); protocol error beyond it. - Result sets claiming more than 4096 columns (the MySQL server hard limit) are protocol errors before any allocation. - All listed sites check remaining length up front and return `Error::Protocol` describing expectation vs. reality; the `unreachable!()` is now an error. ### Is this a breaking change? No: every changed path converts a panic or abort into a returned `Err`, which callers already handle (the pool discards the connection). Newly-observable edges: aggregate packets over 1 GiB and result sets over 4096 columns are now errors rather than allocation attempts. ### Regression tests 15 new unit tests, including a sweep asserting that every strict prefix of captured MySQL 8.0.18 and MariaDB 10.4.7 handshake packets now errors instead of panicking. Each test fails (panics) without the fix. `cargo test -p sqlx-mysql --lib` passes.
portlandhodl
force-pushed
the
security/mysql-decoder-hardening
branch
from
October 14, 2026 04:34
97f2b8f to
404304e
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
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.
Found during a security review of server-controlled parsing paths (threat model: rogue or MITM MySQL/MariaDB server; handshake parsing is reachable pre-authentication).
Does your PR solve an issue?
No tracking issue — reported here directly. This extends the fix pattern of #4176 (truncated
OKpacket) to the rest of the protocol; #4451 documents the same review on the PostgreSQL side.The problem
recv_packet's continuation loop accumulated an unbounded number of0xFFFFFF-sized parts: a hostile server streams endless 16 MiB frames and the client OOMs at near-zero bandwidth cost.HashMap::with_capacity/Vec::reservedirectly; a ~10-byte packet claiming 2^40 columns aborts the process.bytes::Bufgetters / slice indexing /advanceon untrusted buffers: truncated ERR packets, every field ofHandshakeV10(including the attacker-controlled auth-plugin-data length), length-encoded strings claiming more than the remaining bytes,TextRowfield-count mismatch,BinaryRow(null bitmap, temporal length byte, fixed-width and lenenc value bounds — plus anunreachable!()actually reachable via a NULL-typed column), short EOF packets, and a 1-bytecaching_sha2_passwordcontinue packet. The OkPacket decoder was hardened in fix(mysql): return error instead of panic on truncated OK packet #4176; the rest of the protocol was not.The change
MySql's ownmax_allowed_packethard ceiling); protocol error beyond it.Error::Protocoldescribing expectation vs. reality; theunreachable!()is now an error.Is this a breaking change?
No: every changed path converts a panic or abort into a returned
Err, which callers already handle (the pool discards the connection). Newly-observable edges: aggregate packets over 1 GiB and result sets over 4096 columns are now errors rather than allocation attempts.Regression tests
15 new unit tests, including a sweep asserting that every strict prefix of captured MySQL 8.0.18 and MariaDB 10.4.7 handshake packets now errors instead of panicking. Each test fails (panics) without the fix.
cargo test -p sqlx-mysql --libpasses.