Repository navigation
fix(buf): guard against malformed backend frame lengths causing panics - #1451
Closed
AdamMagued wants to merge 1 commit into
Closed
AdamMagued wants to merge 1 commit into
AdamMagued wants to merge 1 commit into
Conversation
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.
Fixes #1447
Problem
When receiving a backend message,
conn.recvMessageextracts the 4-byte message length header and calculatesn := int(binary.BigEndian.Uint32(x[1:])) - 4. If the incoming frame specifies a length less than 4 (including 0, 1, 2, 3) or a negative value when interpreted as a signed 32-bit integer,nbecomes negative.Because
n <= len(cn.scratch)evaluated to true for negative values (e.g.,-4 <= 512), the code executedr.b = cn.scratch[:n], triggering a slice bounds out of range panic (panic: runtime error: slice bounds out of range [:-4]). A similar condition occurred when parsing column lengths inrows.NextforDataRowmessages where invalid column lengths caused slicing panics or out-of-bounds reads.Solution
conn.recvMessage, cast the 4-byte length field to a signed 32-bit integermsgLen := int(int32(binary.BigEndian.Uint32(x[1:]))).n >= 0and thatmsgLendoes not exceed the protocol message limit (1 GB). If invalid, mark the connection withdriver.ErrBadConnand return an error indicating loss of synchronization.rows.Next, validate the column value lengthlagainst 0 andlen(rs.rb)before slicing.buf_test.goand an integration test inconn_test.goverifying that malformed frame lengths (zero, sub-header, negative, and oversized) return descriptive errors and mark the connection as bad without panicking.