bound datetime/time stmt param reads to the declared length#5164
bound datetime/time stmt param reads to the declared length#5164jmestwa-coder wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the length validation checks when parsing TIME and DATETIME parameters in sql/sql_prepare.cc to ensure proper bounds checking before reading time components and fractional seconds. There are no review comments, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution. This is a preliminary review!
This needs:
- a MDEV describing the problem, complete with steps to reproduce
- a commit message compliant with the MariaDB coding standards
- a regression test
- a rebase to the lowest affected version (this is a bug fix and not a feature).
…ength get_param_length() guarantees only `length` bytes for a prepared-statement parameter value sent over the binary protocol. set_param_datetime() reads to[4..6] when length > 4 (needs 7 bytes) and sint4korr(to+7) when length > 7 (needs 11); set_param_time() reads sint4korr(to+8) when length > 8 (needs 12). A client that sends an off-spec datetime length of 5, 6, 8, 9 or 10, or a time length of 9, 10 or 11, makes the server read up to 3 bytes past the validated value, an out-of-bounds read when the value ends at the packet buffer boundary. Gate the optional time and microsecond reads on the canonical encoding lengths (>= 7 and >= 11 for datetime, >= 12 for time) so a short value falls back to the next-shorter form instead of over-reading. Well-formed values (datetime 4/7/11, time 8/12) parse exactly as before.
c737116 to
3eec106
Compare
|
Pushed an update:
How it triggers: get_param_length() only validates On the regression test: the off-spec length isn't reachable through the client library, since it always encodes the canonical lengths (4/7/11 for datetime, 8/12 for time), so mysql_client_test can't drive a short length. I confirmed the bug with a small standalone harness that runs get_param_length() plus the read logic with the value placed flush against a PROT_NONE guard page: the off-spec lengths fault before the patch and exit clean after, while the valid lengths are unchanged. I can attach that harness, or wire up a crafted-packet test if there's a spot you'd prefer for one. I'll open the MDEV with the details above and prefix the commit subject with the id. |
get_param_length() validates only
lengthbytes for a parameter value, but the binary-protocol datetime reader reads to[4..6] when length>4 (needs 7) and to[7..10] when length>7 (needs 11), and the time reader reads to[8..11] when length>8 (needs 12). A client sending an off-spec datetime length of 5/6/8/9/10 or a time length of 9/10/11 makes the server read past the value, out of bounds when it ends at the packet boundary. Gate the optional time and microsecond reads on the canonical encoding lengths (7/11 for datetime, 12 for time).